Re: [PATCH v3 13/21] KVM: selftests: Use KVM's reported TSC KHz as L0's frequency (sanity checked)

[Sean Christopherson <seanjc@google.com>]

On Fri, Oct 02, 2026, Jim Mattson wrote:
> On Wed, Sep 30, 2026 at 11:23 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Wed, Sep 30, 2026, sashiko-bot@kernel.org wrote:
> 
> > > > @@ -212,18 +209,30 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_
> > > >  int main(int argc, char *argv[])
> > > >  {
> > > >     u64 l0_tsc_freq, tsc_start, tsc_end, l1_scale, l2_scale;
> > > > +   struct kvm_vm *vm;
> > > >
> > > >     TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_VMX) ||
> > > >                  kvm_cpu_has(X86_FEATURE_SVM));
> > > >     TEST_REQUIRE(kvm_has_cap(KVM_CAP_TSC_CONTROL));
> > > >     TEST_REQUIRE(sys_clocksource_is_based_on_tsc());
> > > >
> > > > +   /*
> > > > +    * Create a dummy VM to get KVM's default TSC frequency.  All CPUs that
> > > > +    * support TSC scaling should have a constant TSC, i.e. there's no need
> > > > +    * to calibrate the "real" TSC.  But do sanity check that the observed
> > > > +    * TSC is within range of KVM's reported TSC frequency.
> > > > +    */
> > > > +   vm = vm_create_barebones();
> > > > +   l0_tsc_freq = (u64)__vm_ioctl(vm, KVM_GET_TSC_KHZ, NULL) * 1000;
> > > > +   TEST_ASSERT(l0_tsc_freq, "vcpu ioctl KVM_GET_TSC_KHZ failed");
> > >
> > > [Severity: Low]
> > > Does this properly catch an ioctl failure? If __vm_ioctl() fails and returns
> > > -1, casting it to a u64 and multiplying by 1000 results in a very large
> > > non-zero value (0xfffffffffffffc18).
> > >
> > > Since TEST_ASSERT only checks if l0_tsc_freq is non-zero, won't this pass
> > > using garbage data instead of failing the test?
> >
> > Eh, yes and no.  KVM_GET_TSC_KHZ existed long before TSC scaling, and while KVM
> > doesn't sanity check tsc_khz, i.e. KVM_GET_TSC_KHZ could return a "legitimate"
> > negative frequency (when those ~2.17PHz CPUs come along), in practice checking
> > for a non-zero value is sufficient because any bogus value will quickly fail the
> > functional checks.
> >
> > Heh, actually, kvm_arch_vm_post_create() *already* validates KVM_GET_TSC_KHZ:
> >
> >         r = __vm_ioctl(vm, KVM_GET_TSC_KHZ, NULL);
> >         TEST_ASSERT(r > 0, "KVM_GET_TSC_KHZ did not provide a valid TSC frequency.");
> >         guest_tsc_khz = r;
> >
> > so this is a non-issue.  I agree it's not the prettiest, I just don't care enough
> > to clean it up.
> 
> In my best Sashiko passive-aggressive tone:
> 
> Is it possible that vm_create_barebones() bypasses that check?

LOL, *sigh*.  I'll send a patch to add a wrapper, having to manually do the
assert is annoying.