Re: [PATCH v16 10/13] x86/tsc: Switch Secure TSC guests away from kvm-clock

[Nikunj A. Dadhania]

On 1/7/2025 8:46 PM, Borislav Petkov wrote:
> On Mon, Jan 06, 2025 at 06:16:30PM +0530, Nikunj A Dadhania wrote:
>>  static int kvm_cs_enable(struct clocksource *cs)
>>  {
>> +	/*
>> +	 * TSC clocksource should be used for a guest with Secure TSC enabled,
>> +	 * taint the kernel and warn when the user changes the clocksource to
>> +	 * kvm-clock.
>> +	 */
>> +	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) {
>> +		add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
>> +		WARN_ONCE(1, "For Secure TSC guest, changing the clocksource is not allowed!\n");
> 
> So this thing is trying to state that changing the clocksource is not allowed
> but it still allows it. Why not simply do this:
> 
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index 960260a8d884..d8fef3a65a35 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -151,14 +151,10 @@ bool kvm_check_and_clear_guest_paused(void)
>  
>  static int kvm_cs_enable(struct clocksource *cs)
>  {
> -	/*
> -	 * TSC clocksource should be used for a guest with Secure TSC enabled,
> -	 * taint the kernel and warn when the user changes the clocksource to
> -	 * kvm-clock.
> -	 */
> +	/* Only the TSC should be used in a Secure TSC guest. */
>  	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) {
> -		add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
> -		WARN_ONCE(1, "For Secure TSC guest, changing the clocksource is not allowed!\n");
> +		WARN_ONCE(1, "Secure TSC guest, changing the clocksource is not allowed!\n");
> +		return 1;
>  	}
>  
>  	vclocks_set_used(VDSO_CLOCKMODE_PVCLOCK);
> 
> ?

Works as expected:

$ echo 'kvm-clock' > /sys/devices/system/clocksource/clocksource0/current_clocksource
[   30.333603] ------------[ cut here ]------------
[   30.334802] Secure TSC guest, changing the clocksource is not allowed!
[   30.336460] WARNING: CPU: 0 PID: 19 at arch/x86/kernel/kvmclock.c:156 kvm_cs_enable+0x57/0x70

Regards
Nikunj