Re: [PATCH v15 12/13] x86/kvmclock: Abort SecureTSC enabled guest when kvmclock is selected

Nikunj A. Dadhania
2025-01-01T09:44:27Z

On 12/30/2024 10:34 PM, Borislav Petkov wrote:
> On Tue, Dec 03, 2024 at 02:30:44PM +0530, Nikunj A Dadhania wrote:
>> SecureTSC enabled guests should use TSC as the only clock source, terminate
>> the guest with appropriate code when clock source switches to hypervisor
>> controlled kvmclock.
>
> This looks silly.

I can drop this patch, and if the admin wants to change the clock
source to kvm-clock from Secure TSC, that will be permitted.

> Why not prevent kvm_register_clock() from succeeding simply
> on a secure-TSC guest?

Or we can have something like below that will prevent kvm-clock
registration:

diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 39dda04b5ba0..e68683a65018 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -151,14 +151,6 @@ bool kvm_check_and_clear_guest_paused(void)

static int kvm_cs_enable(struct clocksource *cs)
{
- /*
- * For a guest with SecureTSC enabled, the TSC should be the only clock
- * source. Abort the guest when kvmclock is selected as the clock
- * source.
- */
- if (WARN_ON(cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)))
- sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECURE_TSC_KVMCLOCK);
-
vclocks_set_used(VDSO_CLOCKMODE_PVCLOCK);
return 0;
}
@@ -352,6 +344,12 @@ void __init kvmclock_init(void)
boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
!check_tsc_unstable())
kvm_clock.rating = 299;
+ /*
+ * For a guest with SecureTSC enabled, the TSC clock source should be
+ * used. Prevent the kvm-clock source registration.
+ */
+ if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
+ return;

clocksource_register_hz(&kvm_clock, NSEC_PER_SEC);
pv_info.name = "KVM";