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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fb1d32fb-f213-350f-95a4-766c88a6249c@amd.com>
Date: Mon, 3 Feb 2025 08:48:46 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Sean Christopherson <seanjc@...gle.com>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 x86@...nel.org, "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
 Juergen Gross <jgross@...e.com>, "K. Y. Srinivasan" <kys@...rosoft.com>,
 Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
 Dexuan Cui <decui@...rosoft.com>, Ajay Kaher <ajay.kaher@...adcom.com>,
 Alexey Makhalov <alexey.amakhalov@...adcom.com>,
 Jan Kiszka <jan.kiszka@...mens.com>, Paolo Bonzini <pbonzini@...hat.com>,
 Andy Lutomirski <luto@...nel.org>, Peter Zijlstra <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org, linux-coco@...ts.linux.dev,
 virtualization@...ts.linux.dev, linux-hyperv@...r.kernel.org,
 jailhouse-dev@...glegroups.com, kvm@...r.kernel.org,
 xen-devel@...ts.xenproject.org, Nikunj A Dadhania <nikunj@....com>
Subject: Re: [PATCH 08/16] x86/tsc: Pass KNOWN_FREQ and RELIABLE as params to
 registration

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@...gle.com>
> ---
>  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?

Thanks,
Tom

> -	if (vmware_tsc_khz)
> -		setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
> +
>  	if (vmware_hypercall_mode == CPUID_VMWARE_FEATURES_ECX_VMCALL)
>  		setup_force_cpu_cap(X86_FEATURE_VMCALL);
>  	else if (vmware_hypercall_mode == CPUID_VMWARE_FEATURES_ECX_VMMCALL)
> @@ -417,7 +417,8 @@ static void __init vmware_platform_setup(void)
>  
>  		vmware_tsc_khz = tsc_khz;
>  		tsc_register_calibration_routines(vmware_get_tsc_khz,
> -						  vmware_get_tsc_khz);
> +						  vmware_get_tsc_khz,
> +						  TSC_FREQ_KNOWN_AND_RELIABLE);
>  
>  #ifdef CONFIG_X86_LOCAL_APIC
>  		/* Skip lapic calibration since we know the bus frequency. */