[v12] Add Secure TSC support for SNP guests

[PATCH v12 17/19] x86/kvmclock: Abort SecureTSC enabled guest when kvmclock is selected

Posted by Nikunj A Dadhania 1 month, 2 weeks ago

SecureTSC enabled guests should use TSC as the only clock source, abort
the guest when clock source switches to hypervisor controlled kvmclock.

Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 arch/x86/kernel/kvmclock.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 5cd3717e103b..552c28cda874 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -22,6 +22,7 @@
 #include <asm/x86_init.h>
 #include <asm/kvmclock.h>
 #include <asm/timer.h>
+#include <asm/sev.h>
 
 static int kvmclock __initdata = 1;
 static int kvmclock_vsyscall __initdata = 1;
@@ -155,6 +156,13 @@ static void enable_kvm_sc_work(struct work_struct *work)
 {
 	u8 flags;
 
+	/*
+	 * For guest with SecureTSC enabled, TSC should be the only clock source.
+	 * Abort the guest when kvmclock is selected as the clock source.
+	 */
+	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
+		snp_abort();
+
 	old_pv_sched_clock = static_call_query(pv_sched_clock);
 	flags = pvclock_read_flags(&hv_clock_boot[0].pvti);
 	kvm_sched_clock_init(flags & PVCLOCK_TSC_STABLE_BIT);
-- 
2.34.1

Re: [PATCH v12 17/19] x86/kvmclock: Abort SecureTSC enabled guest when kvmclock is selected

Posted by Tom Lendacky 1 month, 2 weeks ago

On 10/9/24 04:28, Nikunj A Dadhania wrote:
> SecureTSC enabled guests should use TSC as the only clock source, abort
> the guest when clock source switches to hypervisor controlled kvmclock.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
>  arch/x86/kernel/kvmclock.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index 5cd3717e103b..552c28cda874 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -22,6 +22,7 @@
>  #include <asm/x86_init.h>
>  #include <asm/kvmclock.h>
>  #include <asm/timer.h>
> +#include <asm/sev.h>
>  
>  static int kvmclock __initdata = 1;
>  static int kvmclock_vsyscall __initdata = 1;
> @@ -155,6 +156,13 @@ static void enable_kvm_sc_work(struct work_struct *work)
>  {
>  	u8 flags;
>  
> +	/*
> +	 * For guest with SecureTSC enabled, TSC should be the only clock source.

s/For guest/For a guest/
s/TSC should/The TSC should/

> +	 * Abort the guest when kvmclock is selected as the clock source.
> +	 */
> +	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
> +		snp_abort();

Can a message be issued here?

Also, you could use sev_es_terminate() to provide a specific Linux
reason code, e.g.:

  sev_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECURE_TSC_KVMCLOCK);

or whatever name you want to use for this situation.

Thanks,
Tom

> +
>  	old_pv_sched_clock = static_call_query(pv_sched_clock);
>  	flags = pvclock_read_flags(&hv_clock_boot[0].pvti);
>  	kvm_sched_clock_init(flags & PVCLOCK_TSC_STABLE_BIT);

Re: [PATCH v12 17/19] x86/kvmclock: Abort SecureTSC enabled guest when kvmclock is selected

Posted by Nikunj A. Dadhania 1 month, 2 weeks ago


On 10/11/2024 1:19 AM, Tom Lendacky wrote:
> On 10/9/24 04:28, Nikunj A Dadhania wrote:
>> SecureTSC enabled guests should use TSC as the only clock source, abort
>> the guest when clock source switches to hypervisor controlled kvmclock.
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
>> ---
>>  arch/x86/kernel/kvmclock.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
>> index 5cd3717e103b..552c28cda874 100644
>> --- a/arch/x86/kernel/kvmclock.c
>> +++ b/arch/x86/kernel/kvmclock.c
>> @@ -22,6 +22,7 @@
>>  #include <asm/x86_init.h>
>>  #include <asm/kvmclock.h>
>>  #include <asm/timer.h>
>> +#include <asm/sev.h>
>>  
>>  static int kvmclock __initdata = 1;
>>  static int kvmclock_vsyscall __initdata = 1;
>> @@ -155,6 +156,13 @@ static void enable_kvm_sc_work(struct work_struct *work)
>>  {
>>  	u8 flags;
>>  
>> +	/*
>> +	 * For guest with SecureTSC enabled, TSC should be the only clock source.
> 
> s/For guest/For a guest/
> s/TSC should/The TSC should/

Ok

> 
>> +	 * Abort the guest when kvmclock is selected as the clock source.
>> +	 */
>> +	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
>> +		snp_abort();
> 
> Can a message be issued here?
> 
> Also, you could use sev_es_terminate() to provide a specific Linux
> reason code, e.g.:
> 
>   sev_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECURE_TSC_KVMCLOCK);
> 
> or whatever name you want to use for this situation.

Sure, will add.

Regards
Nikunj