mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Jim Mattson <jmattson@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>,
	Sashiko Bot <sashiko-bot@kernel.org>
Subject: Re: [PATCH v3 13/21] KVM: selftests: Use KVM's reported TSC KHz as L0's frequency (sanity checked)
Date: Tue, 6 Oct 2026 23:04:13 -0700	[thread overview]
Message-ID: <asXg3cJvaeqMOfAr@google.com> (raw)
In-Reply-To: <CALMp9eS0=HgM02Ff4mv6RLHVoM3EgavhKu_P6J-oTWTMbk9zng@mail.gmail.com>

On Tue, Oct 06, 2026, Jim Mattson wrote:
> On Wed, Sep 30, 2026 at 12:04 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Use KVM's reported default TSC frequency as L0's frequency instead of
> > calibrating L0's frequency using RDTSC.  Trying to calibrate the frequency
> > in userspace is all but guaranteed to be wildly inaccurate, e.g. due to
> > interrupts, preemption, etc, and no known CPU supports TSC scaling without
> > also having a constant TSC.  I.e. there's no need to calibrate the current
> > frequency, as it should also be the same as KVM's default frequency.
> >
> > Opportunistically sanity check KVM's reported frequency against what is
> > observed via RDTSC, e.g. so that KVM_GET_TSC_KHZ or platform issues show up
> > much earlier in the test, before VMs get involved.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  .../kvm/x86/nested_tsc_scaling_test.c         | 25 +++++++++++++------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
> > index d790ab36a198..a78585af0ffd 100644
> > --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
> > +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
> > @@ -137,7 +137,7 @@ static void l1_guest_code(void *data)
> >
> >  static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_factor)
> >  {
> > -       u64 tsc_khz, l1_tsc_freq, l2_tsc_freq;
> > +       u64 l1_tsc_freq, l2_tsc_freq;
> >         struct kvm_vcpu *vcpu;
> >         struct kvm_vm *vm;
> >         gva_t guest_gva;
> > @@ -159,11 +159,8 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_
> >
> >         vcpu_args_set(vcpu, 1, guest_gva);
> >
> > -       tsc_khz = __vcpu_ioctl(vcpu, KVM_GET_TSC_KHZ, NULL);
> > -       TEST_ASSERT(tsc_khz != -1, "vcpu ioctl KVM_GET_TSC_KHZ failed");
> > -
> >         /* scale down L1's TSC frequency */
> > -       vcpu_ioctl(vcpu, KVM_SET_TSC_KHZ, (void *) (tsc_khz / l1_scale_factor));
> > +       vcpu_ioctl(vcpu, KVM_SET_TSC_KHZ, (void *) (l0_tsc_freq / l1_scale_factor));
> 
> Our internal Sashiko complains:
> 
> [Severity: Medium]
> Is l0_tsc_freq passed to KVM_SET_TSC_KHZ in Hz rather than KHz?
> 
> In main(), l0_tsc_freq is calculated in Hz by multiplying the KVM_GET_TSC_KHZ
> result by 1000. Passing this value divided by l1_scale_factor into
> KVM_SET_TSC_KHZ configures the guest TSC frequency 1000x larger than
> intended, since the ioctl expects a value in KHz.
> 
> I concur.

Drat, I thought I had fixed all of those goofs.

  reply	other threads:[~2026-10-07  6:04 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30 17:36 [PATCH v3 00/21] KVM: x86: Fix nested TSC scaling edge cases Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 01/21] KVM: x86: Saturate L2's TSC frequency if it exceeds hardware supports Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 02/21] KVM: SVM: Fallback to the default TSC ratio if KVM tries to use a bad multiplier Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 03/21] KVM: x86: Allow userspace to set KVM's max supported guest TSC frequency Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 04/21] KVM: selftests: Use KVM's pRNG to randomize L1's TSC ratio in TSC scaling test Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 05/21] KVM: selftests: Drop redundant VMWRITE of TSC_MULTIPLIER_HIGH Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 06/21] KVM: selftests: Drop unnecessary use of PRIu64 in nested TSC scaling test Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 07/21] KVM: selftests: Randomize L2's scale factor " Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 08/21] KVM: selftests: Rename TSC freq checkers " Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 09/21] KVM: selftests: Extract guts of nested TSC scaling test to helper function Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 10/21] KVM: selftests: Track L2 multiplier, not scale-up factor, in nested TSC test Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 11/21] KVM: selftests: Print out the failing L{0,1,2} level in nested TSC scaling test Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 12/21] KVM: selftests: Allow +/- 1 tolerance if expected TSC frequency is <100 Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 13/21] KVM: selftests: Use KVM's reported TSC KHz as L0's frequency (sanity checked) Sean Christopherson
2026-10-06 21:57   ` Jim Mattson
2026-10-07  6:04     ` Sean Christopherson [this message]
2026-10-07 17:25       ` Jim Mattson
2026-09-30 17:36 ` [PATCH v3 14/21] KVM: selftests: Explicitly pass TSC frequencies to guts of TSC scaling test Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 15/21] KVM: selftests: Sanity check KVM's default TSC freq in nested " Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 16/21] KVM: selftests: Test L1 "up" and L2 "down" " Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 17/21] KVM: selftests: Verify that KVM saturates L2 TSC freq on {under,over}flow Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 18/21] KVM: selftests: Test non-zero TSC offset on SVM in nested TSC scaling test Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 19/21] KVM: selftests: Randomize L2's TSC offset in the " Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 20/21] KVM: selftests: Use GUEST_SYNC2() in " Sean Christopherson
2026-09-30 17:36 ` [PATCH v3 21/21] KVM: selftests: Spell out UCALL in nested TSC scaling test's enums Sean Christopherson
2026-10-02 21:07 ` [PATCH v3 00/21] KVM: x86: Fix nested TSC scaling edge cases 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=asXg3cJvaeqMOfAr@google.com \
    --to=seanjc@google.com \
    --cc=amirmohammad.eftekhar@cispa.de \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=sashiko-bot@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

all inboxes | Powered by JetHome®