1. Patchew
  2. linux
  3. [PATCH v13 00/13] Add Secure TSC support for SNP guests
  4. View patch

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

Nikunj A Dadhania posted 13 patches 1 month ago
There is a newer version of this series
[PATCH v13 12/13] x86/kvmclock: Abort SecureTSC enabled guest when kvmclock is selected
Posted by Nikunj A Dadhania 1 month ago 
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.

Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 arch/x86/include/asm/sev-common.h | 1 +
 arch/x86/include/asm/sev.h        | 2 ++
 arch/x86/coco/sev/shared.c        | 3 +--
 arch/x86/kernel/kvmclock.c        | 9 +++++++++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index 6ef92432a5ce..ad0743800b0e 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -207,6 +207,7 @@ struct snp_psc_desc {
 #define GHCB_TERM_SVSM_VMPL0		8	/* SVSM is present but has set VMPL to 0 */
 #define GHCB_TERM_SVSM_CAA		9	/* SVSM is present but CAA is not page aligned */
 #define GHCB_TERM_SECURE_TSC		10	/* Secure TSC initialization failed */
+#define GHCB_TERM_SECURE_TSC_KVMCLOCK	11	/* KVM clock selected instead of Secure TSC */
 
 #define GHCB_RESP_CODE(v)		((v) & GHCB_MSR_INFO_MASK)
 
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 34f7b9fc363b..783dc57f73c3 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -537,6 +537,7 @@ static inline int handle_guest_request(struct snp_msg_desc *mdesc, u64 exit_code
 
 void __init snp_secure_tsc_prepare(void);
 void __init securetsc_init(void);
+void __noreturn sev_es_terminate(unsigned int set, unsigned int reason);
 
 #else	/* !CONFIG_AMD_MEM_ENCRYPT */
 
@@ -586,6 +587,7 @@ static inline int handle_guest_request(struct snp_msg_desc *mdesc, u64 exit_code
 
 static inline void __init snp_secure_tsc_prepare(void) { }
 static inline void __init securetsc_init(void) { }
+static inline void sev_es_terminate(unsigned int set, unsigned int reason) { }
 
 #endif	/* CONFIG_AMD_MEM_ENCRYPT */
 
diff --git a/arch/x86/coco/sev/shared.c b/arch/x86/coco/sev/shared.c
index c2a9e2ada659..d202790e1385 100644
--- a/arch/x86/coco/sev/shared.c
+++ b/arch/x86/coco/sev/shared.c
@@ -117,8 +117,7 @@ static bool __init sev_es_check_cpu_features(void)
 	return true;
 }
 
-static void __head __noreturn
-sev_es_terminate(unsigned int set, unsigned int reason)
+void __head __noreturn sev_es_terminate(unsigned int set, unsigned int reason)
 {
 	u64 val = GHCB_MSR_TERM_REQ;
 
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 5b2c15214a6b..b135044f3c7b 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -21,6 +21,7 @@
 #include <asm/hypervisor.h>
 #include <asm/x86_init.h>
 #include <asm/kvmclock.h>
+#include <asm/sev.h>
 
 static int kvmclock __initdata = 1;
 static int kvmclock_vsyscall __initdata = 1;
@@ -150,6 +151,14 @@ 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 (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;
 }
-- 
2.34.1
Re: [PATCH v13 12/13] x86/kvmclock: Abort SecureTSC enabled guest when kvmclock is selected
Posted by Tom Lendacky 1 month ago 
On 10/21/24 00:51, 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.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
>  arch/x86/include/asm/sev-common.h | 1 +
>  arch/x86/include/asm/sev.h        | 2 ++
>  arch/x86/coco/sev/shared.c        | 3 +--
>  arch/x86/kernel/kvmclock.c        | 9 +++++++++
>  4 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
> index 6ef92432a5ce..ad0743800b0e 100644
> --- a/arch/x86/include/asm/sev-common.h
> +++ b/arch/x86/include/asm/sev-common.h
> @@ -207,6 +207,7 @@ struct snp_psc_desc {
>  #define GHCB_TERM_SVSM_VMPL0		8	/* SVSM is present but has set VMPL to 0 */
>  #define GHCB_TERM_SVSM_CAA		9	/* SVSM is present but CAA is not page aligned */
>  #define GHCB_TERM_SECURE_TSC		10	/* Secure TSC initialization failed */
> +#define GHCB_TERM_SECURE_TSC_KVMCLOCK	11	/* KVM clock selected instead of Secure TSC */
>  
>  #define GHCB_RESP_CODE(v)		((v) & GHCB_MSR_INFO_MASK)
>  
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 34f7b9fc363b..783dc57f73c3 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -537,6 +537,7 @@ static inline int handle_guest_request(struct snp_msg_desc *mdesc, u64 exit_code
>  
>  void __init snp_secure_tsc_prepare(void);
>  void __init securetsc_init(void);
> +void __noreturn sev_es_terminate(unsigned int set, unsigned int reason);
>  
>  #else	/* !CONFIG_AMD_MEM_ENCRYPT */
>  
> @@ -586,6 +587,7 @@ static inline int handle_guest_request(struct snp_msg_desc *mdesc, u64 exit_code
>  
>  static inline void __init snp_secure_tsc_prepare(void) { }
>  static inline void __init securetsc_init(void) { }
> +static inline void sev_es_terminate(unsigned int set, unsigned int reason) { }
>  
>  #endif	/* CONFIG_AMD_MEM_ENCRYPT */
>  
> diff --git a/arch/x86/coco/sev/shared.c b/arch/x86/coco/sev/shared.c
> index c2a9e2ada659..d202790e1385 100644
> --- a/arch/x86/coco/sev/shared.c
> +++ b/arch/x86/coco/sev/shared.c
> @@ -117,8 +117,7 @@ static bool __init sev_es_check_cpu_features(void)
>  	return true;
>  }
>  
> -static void __head __noreturn
> -sev_es_terminate(unsigned int set, unsigned int reason)
> +void __head __noreturn sev_es_terminate(unsigned int set, unsigned int reason)
>  {
>  	u64 val = GHCB_MSR_TERM_REQ;
>  
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index 5b2c15214a6b..b135044f3c7b 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -21,6 +21,7 @@
>  #include <asm/hypervisor.h>
>  #include <asm/x86_init.h>
>  #include <asm/kvmclock.h>
> +#include <asm/sev.h>
>  
>  static int kvmclock __initdata = 1;
>  static int kvmclock_vsyscall __initdata = 1;
> @@ -150,6 +151,14 @@ 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 (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))

if (WARN_ON(cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)))

so that the guest sees something as well?

Thanks,
Tom

> +		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECURE_TSC_KVMCLOCK);
> +
>  	vclocks_set_used(VDSO_CLOCKMODE_PVCLOCK);
>  	return 0;
>  }
Re: [PATCH v13 12/13] x86/kvmclock: Abort SecureTSC enabled guest when kvmclock is selected
Posted by Nikunj A. Dadhania 1 month ago 
On 10/21/2024 8:30 PM, Tom Lendacky wrote:
> On 10/21/24 00:51, 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.
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
>> ---
>>  arch/x86/include/asm/sev-common.h | 1 +
>>  arch/x86/include/asm/sev.h        | 2 ++
>>  arch/x86/coco/sev/shared.c        | 3 +--
>>  arch/x86/kernel/kvmclock.c        | 9 +++++++++
>>  4 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
>> index 6ef92432a5ce..ad0743800b0e 100644
>> --- a/arch/x86/include/asm/sev-common.h
>> +++ b/arch/x86/include/asm/sev-common.h
>> @@ -207,6 +207,7 @@ struct snp_psc_desc {
>>  #define GHCB_TERM_SVSM_VMPL0		8	/* SVSM is present but has set VMPL to 0 */
>>  #define GHCB_TERM_SVSM_CAA		9	/* SVSM is present but CAA is not page aligned */
>>  #define GHCB_TERM_SECURE_TSC		10	/* Secure TSC initialization failed */
>> +#define GHCB_TERM_SECURE_TSC_KVMCLOCK	11	/* KVM clock selected instead of Secure TSC */
>>  
>>  #define GHCB_RESP_CODE(v)		((v) & GHCB_MSR_INFO_MASK)
>>  
>> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
>> index 34f7b9fc363b..783dc57f73c3 100644
>> --- a/arch/x86/include/asm/sev.h
>> +++ b/arch/x86/include/asm/sev.h
>> @@ -537,6 +537,7 @@ static inline int handle_guest_request(struct snp_msg_desc *mdesc, u64 exit_code
>>  
>>  void __init snp_secure_tsc_prepare(void);
>>  void __init securetsc_init(void);
>> +void __noreturn sev_es_terminate(unsigned int set, unsigned int reason);
>>  
>>  #else	/* !CONFIG_AMD_MEM_ENCRYPT */
>>  
>> @@ -586,6 +587,7 @@ static inline int handle_guest_request(struct snp_msg_desc *mdesc, u64 exit_code
>>  
>>  static inline void __init snp_secure_tsc_prepare(void) { }
>>  static inline void __init securetsc_init(void) { }
>> +static inline void sev_es_terminate(unsigned int set, unsigned int reason) { }
>>  
>>  #endif	/* CONFIG_AMD_MEM_ENCRYPT */
>>  
>> diff --git a/arch/x86/coco/sev/shared.c b/arch/x86/coco/sev/shared.c
>> index c2a9e2ada659..d202790e1385 100644
>> --- a/arch/x86/coco/sev/shared.c
>> +++ b/arch/x86/coco/sev/shared.c
>> @@ -117,8 +117,7 @@ static bool __init sev_es_check_cpu_features(void)
>>  	return true;
>>  }
>>  
>> -static void __head __noreturn
>> -sev_es_terminate(unsigned int set, unsigned int reason)
>> +void __head __noreturn sev_es_terminate(unsigned int set, unsigned int reason)
>>  {
>>  	u64 val = GHCB_MSR_TERM_REQ;
>>  
>> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
>> index 5b2c15214a6b..b135044f3c7b 100644
>> --- a/arch/x86/kernel/kvmclock.c
>> +++ b/arch/x86/kernel/kvmclock.c
>> @@ -21,6 +21,7 @@
>>  #include <asm/hypervisor.h>
>>  #include <asm/x86_init.h>
>>  #include <asm/kvmclock.h>
>> +#include <asm/sev.h>
>>  
>>  static int kvmclock __initdata = 1;
>>  static int kvmclock_vsyscall __initdata = 1;
>> @@ -150,6 +151,14 @@ 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 (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
> 
> if (WARN_ON(cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)))
> 
> so that the guest sees something as well?

Sure, will add.

Regards
Nikunj

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.