Re: [PATCH 08/16] x86/tsc: Pass KNOWN_FREQ and RELIABLE as params to registration

[Sean Christopherson]

On Mon, Feb 03, 2025, Tom Lendacky wrote:
> On 1/31/25 20:17, Sean Christopherson wrote:
> > Add a "tsc_properties" set of flags and use it to annotate whether the
> > TSC operates at a known and/or reliable frequency when registering a
> > paravirtual TSC calibration routine.  Currently, each PV flow manually
> > sets the associated feature flags, but often in haphazard fashion that
> > makes it difficult for unfamiliar readers to see the properties of the
> > TSC when running under a particular hypervisor.
> > 
> > The other, bigger issue with manually setting the feature flags is that
> > it decouples the flags from the calibration routine.  E.g. in theory, PV
> > code could mark the TSC as having a known frequency, but then have its
> > PV calibration discarded in favor of a method that doesn't use that known
> > frequency.  Passing the TSC properties along with the calibration routine
> > will allow adding sanity checks to guard against replacing a "better"
> > calibration routine with a "worse" routine.
> > 
> > As a bonus, the flags also give developers working on new PV code a heads
> > up that they should at least mark the TSC as having a known frequency.
> > 
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > ---
> >  arch/x86/coco/sev/core.c       |  6 ++----
> >  arch/x86/coco/tdx/tdx.c        |  7 ++-----
> >  arch/x86/include/asm/tsc.h     |  8 +++++++-
> >  arch/x86/kernel/cpu/acrn.c     |  4 ++--
> >  arch/x86/kernel/cpu/mshyperv.c | 10 +++++++---
> >  arch/x86/kernel/cpu/vmware.c   |  7 ++++---
> >  arch/x86/kernel/jailhouse.c    |  4 ++--
> >  arch/x86/kernel/kvmclock.c     |  4 ++--
> >  arch/x86/kernel/tsc.c          |  8 +++++++-
> >  arch/x86/xen/time.c            |  4 ++--
> >  10 files changed, 37 insertions(+), 25 deletions(-)
> > 
> 
> > diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> > index d6f079a75f05..6e4a2053857c 100644
> > --- a/arch/x86/kernel/cpu/vmware.c
> > +++ b/arch/x86/kernel/cpu/vmware.c
> > @@ -385,10 +385,10 @@ static void __init vmware_paravirt_ops_setup(void)
> >   */
> >  static void __init vmware_set_capabilities(void)
> >  {
> > +	/* TSC is non-stop and reliable even if the frequency isn't known. */
> >  	setup_force_cpu_cap(X86_FEATURE_CONSTANT_TSC);
> >  	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> 
> Should this line be deleted, too, or does the VMware flow require this
> to be done separate from the tsc_register_calibration_routines() call?

No idea, I just didn't want to break existing setups.  I assume VMware hypervisors
will always advertise the TSC frequency, but nothing in the code guarantees that.

The check on the hypervisor providing the TSC frequency has existed since the
original support was added, and the CONSTANT+RELIABLE logic was added immediately
after.  So even if it the above code _shouldn't_ be needed, I don't want to be
the sucker that finds out :-)

  395628ef4ea12ff0748099f145363b5e33c69acb x86: Skip verification by the watchdog for TSC clocksource.
  eca0cd028bdf0f6aaceb0d023e9c7501079a7dda x86: Add a synthetic TSC_RELIABLE feature bit.
  88b094fb8d4fe43b7025ea8d487059e8813e02cd x86: Hypervisor detection and get tsc_freq from hypervisor