linux-kernel - Re: [PATCH v16 12/13] x86/tsc: Switch to native sched clock

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: <20250117202848.GAZ4q9gMHorhVMfvM0@fat_crate.local>
Date: Fri, 17 Jan 2025 21:28:48 +0100
From: Borislav Petkov <bp@...en8.de>
To: Sean Christopherson <seanjc@...gle.com>
Cc: "Nikunj A. Dadhania" <nikunj@....com>, linux-kernel@...r.kernel.org,
	thomas.lendacky@....com, x86@...nel.org, kvm@...r.kernel.org,
	mingo@...hat.com, tglx@...utronix.de, dave.hansen@...ux.intel.com,
	pgonda@...gle.com, pbonzini@...hat.com, francescolavra.fl@...il.com,
	Alexey Makhalov <alexey.makhalov@...adcom.com>,
	Juergen Gross <jgross@...e.com>,
	Boris Ostrovsky <boris.ostrovsky@...cle.com>
Subject: Re: [PATCH v16 12/13] x86/tsc: Switch to native sched clock

On Thu, Jan 16, 2025 at 08:56:25AM -0800, Sean Christopherson wrote:
> It's only with SNP and TDX that the clocksource becomes at all interesting.

So basically you're saying, let's just go ahead and trust the TSC when the HV
sets a bunch of CPUID bits.

But we really really trust it when the guest type is SNP+STSC or TDX since
there the HV is out of the picture and the only one who can flub it there is
the OEM.

> CPUID 0x15 (and 0x16?) is guaranteed to be available under TDX, and Secure TSC
> would ideally assert that the kernel doesn't switch to some other calibration
> method too.  Not sure where to hook into that though, without bleeding TDX and
> SNP details everywhere.

We could use the platform calibrate* function pointers and assign TDX- or
SNP-specific ones and perhaps even define new such function ptrs. That's what
the platform stuff is for... needs staring, ofc.

> I agree the naming is weird, but outside of the vendor checks, the VM code is
> identical to the "native" code, so I don't know that it's worth splitting into
> multiple functions.
> 
> What if we simply rename it to calibrate_tsc_from_cpuid()?

This is all wrong layering with all those different guest types having their
own ->calibrate_tsc:

arch/x86/kernel/cpu/acrn.c:32:  x86_platform.calibrate_tsc = acrn_get_tsc_khz;
arch/x86/kernel/cpu/mshyperv.c:424:             x86_platform.calibrate_tsc = hv_get_tsc_khz;
arch/x86/kernel/cpu/vmware.c:419:               x86_platform.calibrate_tsc = vmware_get_tsc_khz;
arch/x86/kernel/jailhouse.c:213:        x86_platform.calibrate_tsc              = jailhouse_get_tsc;
arch/x86/kernel/kvmclock.c:323: x86_platform.calibrate_tsc = kvm_get_tsc_khz;
arch/x86/kernel/tsc.c:944:      tsc_khz = x86_platform.calibrate_tsc();
arch/x86/kernel/tsc.c:1458:                     tsc_khz = x86_platform.calibrate_tsc();
arch/x86/kernel/x86_init.c:148: .calibrate_tsc                  = native_calibrate_tsc,
arch/x86/xen/time.c:569:        x86_platform.calibrate_tsc = xen_tsc_khz;

What you want sounds like a redesign to me considering how you want to keep
the KVM guest code and baremetal pretty close... Hmmm, needs staring...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette