[PATCH v3 0/5] Enable Secure TSC for SEV-SNP

[PATCH v3 0/5] Enable Secure TSC for SEV-SNP

[Nikunj A Dadhania]

The hypervisor controls TSC value calculations for the guest. A malicious
hypervisor can prevent the guest from progressing. The Secure TSC feature for
SEV-SNP allows guests to securely use the RDTSC and RDTSCP instructions. This
ensures the guest has a consistent view of time and prevents a malicious
hypervisor from manipulating time, such as making it appear to move backward or
advance too quickly. For more details, refer to the "Secure Nested Paging
(SEV-SNP)" section, subsection "Secure TSC" in APM Volume 2.

This patch set is also available at:

  https://github.com/AMDESE/linux-kvm/tree/sectsc-host-latest

and is based on kvm-x86/next

Testing Secure TSC
-----------------

Secure TSC guest patches are available as part of v6.14-rc1.

QEMU changes:
https://github.com/nikunjad/qemu/tree/snp-securetsc-latest

QEMU command line SEV-SNP with Secure TSC:

  qemu-system-x86_64 -cpu EPYC-Milan-v2 -smp 4 \
    -object memory-backend-memfd,id=ram1,size=1G,share=true,prealloc=false,reserve=false \
    -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,secure-tsc=on,stsc-freq=2000000000 \
    -machine q35,confidential-guest-support=sev0,memory-backend=ram1 \
    ...

Changelog:
----------
v3:
* Rebased on top of kvm-x86/next
* Collect Acked-by
* Separate patch to add missing desired_tsc_khz field (Tom)
* Invoke kvm_set_msr_common() for non-SecureTSC guests (Tom)
* To align desired_tsc_khz to 4-byte boundary, move the 2-byte pad0 above it (Tom)
* Update commit logs (Tom, Sean)

v2: https://lore.kernel.org/all/20250210092230.151034-1-nikunj@amd.com/
* Address cpufeatures comment from Boris
* Squashed Secure TSC enablement and setting frequency patch
* Set the default TSC KHz for proper calculation of guest offset/multiplier

Ketan Chaturvedi (1):
  KVM: SVM: Enable Secure TSC for SNP guests

Nikunj A Dadhania (4):
  x86/cpufeatures: Add SNP Secure TSC
  crypto: ccp: Add missing member in SNP_LAUNCH_START command structure
  KVM: SVM: Add GUEST_TSC_FREQ MSR for Secure TSC enabled guests
  KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled

 arch/x86/include/asm/cpufeatures.h |  1 +
 arch/x86/include/asm/svm.h         |  1 +
 arch/x86/include/uapi/asm/kvm.h    |  3 ++-
 arch/x86/kvm/svm/sev.c             | 22 ++++++++++++++++++++++
 arch/x86/kvm/svm/svm.c             | 15 +++++++++++++++
 arch/x86/kvm/svm/svm.h             | 11 ++++++++++-
 include/linux/psp-sev.h            |  2 ++
 7 files changed, 53 insertions(+), 2 deletions(-)


base-commit: fed48e2967f402f561d80075a20c5c9e16866e53
-- 
2.43.0

[PATCH v3 1/5] x86/cpufeatures: Add SNP Secure TSC

[Nikunj A Dadhania]

The Secure TSC feature for SEV-SNP allows guests to securely use the RDTSC
and RDTSCP instructions, ensuring that the parameters used cannot be
altered by the hypervisor once the guest is launched. For more details,
refer to the AMD64 APM Vol 2, Section "Secure TSC".

Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
Acked-by: Borislav Petkov (AMD) <bp@alien8.de>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 508c0dad116b..921ed26b0be7 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -448,6 +448,7 @@
 #define X86_FEATURE_VM_PAGE_FLUSH	(19*32+ 2) /* VM Page Flush MSR is supported */
 #define X86_FEATURE_SEV_ES		(19*32+ 3) /* "sev_es" Secure Encrypted Virtualization - Encrypted State */
 #define X86_FEATURE_SEV_SNP		(19*32+ 4) /* "sev_snp" Secure Encrypted Virtualization - Secure Nested Paging */
+#define X86_FEATURE_SNP_SECURE_TSC	(19*32+ 8) /* SEV-SNP Secure TSC */
 #define X86_FEATURE_V_TSC_AUX		(19*32+ 9) /* Virtual TSC_AUX */
 #define X86_FEATURE_SME_COHERENT	(19*32+10) /* hardware-enforced cache coherency */
 #define X86_FEATURE_DEBUG_SWAP		(19*32+14) /* "debug_swap" SEV-ES full debug state swap support */
-- 
2.43.0

[PATCH v3 2/5] crypto: ccp: Add missing member in SNP_LAUNCH_START command structure

[Nikunj A Dadhania]

The sev_data_snp_launch_start structure should include a 4-byte
desired_tsc_khz field before the gosvw field, which was missed in the
initial implementation. As a result, the structure is 4 bytes shorter than
expected by the firmware, causing the gosvw field to start 4 bytes early.
Fix this by adding the missing 4-byte member for the desired TSC frequency.

Fixes: 3a45dc2b419e ("crypto: ccp: Define the SEV-SNP commands")
Cc: stable@vger.kernel.org
Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 include/linux/psp-sev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index f3cad182d4ef..1f3620aaa4e7 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -594,6 +594,7 @@ struct sev_data_snp_addr {
  * @imi_en: launch flow is launching an IMI (Incoming Migration Image) for the
  *          purpose of guest-assisted migration.
  * @rsvd: reserved
+ * @desired_tsc_khz: hypervisor desired mean TSC freq in kHz of the guest
  * @gosvw: guest OS-visible workarounds, as defined by hypervisor
  */
 struct sev_data_snp_launch_start {
@@ -603,6 +604,7 @@ struct sev_data_snp_launch_start {
 	u32 ma_en:1;				/* In */
 	u32 imi_en:1;				/* In */
 	u32 rsvd:30;
+	u32 desired_tsc_khz;			/* In */
 	u8 gosvw[16];				/* In */
 } __packed;
 
-- 
2.43.0

[PATCH v3 3/5] KVM: SVM: Add GUEST_TSC_FREQ MSR for Secure TSC enabled guests

[Nikunj A Dadhania]

Introduce the read-only MSR GUEST_TSC_FREQ (0xc0010134) that returns
guest's effective frequency in MHZ when Secure TSC is enabled for SNP
guests. Disable interception of this MSR when Secure TSC is enabled. Note
that GUEST_TSC_FREQ MSR is accessible only to the guest and not from the
hypervisor context.

Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 arch/x86/include/asm/svm.h |  1 +
 arch/x86/kvm/svm/sev.c     |  2 ++
 arch/x86/kvm/svm/svm.c     |  1 +
 arch/x86/kvm/svm/svm.h     | 11 ++++++++++-
 4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index e2fac21471f5..a04346068c60 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -289,6 +289,7 @@ static_assert((X2AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AVIC_
 #define SVM_SEV_FEAT_RESTRICTED_INJECTION		BIT(3)
 #define SVM_SEV_FEAT_ALTERNATE_INJECTION		BIT(4)
 #define SVM_SEV_FEAT_DEBUG_SWAP				BIT(5)
+#define SVM_SEV_FEAT_SECURE_TSC				BIT(9)
 
 #define SVM_SEV_FEAT_INT_INJ_MODES		\
 	(SVM_SEV_FEAT_RESTRICTED_INJECTION |	\
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 74525651770a..7875bb14a2b1 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -843,6 +843,8 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
 	save->dr6  = svm->vcpu.arch.dr6;
 
 	save->sev_features = sev->vmsa_features;
+	if (snp_secure_tsc_enabled(vcpu->kvm))
+		set_msr_interception(&svm->vcpu, svm->msrpm, MSR_AMD64_GUEST_TSC_FREQ, 1, 1);
 
 	/*
 	 * Skip FPU and AVX setup with KVM_SEV_ES_INIT to avoid
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index b8aa0f36850f..93cf508f983c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -143,6 +143,7 @@ static const struct svm_direct_access_msrs {
 	{ .index = X2APIC_MSR(APIC_TMICT),		.always = false },
 	{ .index = X2APIC_MSR(APIC_TMCCT),		.always = false },
 	{ .index = X2APIC_MSR(APIC_TDCR),		.always = false },
+	{ .index = MSR_AMD64_GUEST_TSC_FREQ,		.always = false },
 	{ .index = MSR_INVALID,				.always = false },
 };
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 5b159f017055..7335af2ab1df 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -44,7 +44,7 @@ static inline struct page *__sme_pa_to_page(unsigned long pa)
 #define	IOPM_SIZE PAGE_SIZE * 3
 #define	MSRPM_SIZE PAGE_SIZE * 2
 
-#define MAX_DIRECT_ACCESS_MSRS	48
+#define MAX_DIRECT_ACCESS_MSRS	49
 #define MSRPM_OFFSETS	32
 extern u32 msrpm_offsets[MSRPM_OFFSETS] __read_mostly;
 extern bool npt_enabled;
@@ -377,10 +377,19 @@ static __always_inline bool sev_snp_guest(struct kvm *kvm)
 	return (sev->vmsa_features & SVM_SEV_FEAT_SNP_ACTIVE) &&
 	       !WARN_ON_ONCE(!sev_es_guest(kvm));
 }
+
+static inline bool snp_secure_tsc_enabled(struct kvm *kvm)
+{
+	struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
+
+	return (sev->vmsa_features & SVM_SEV_FEAT_SECURE_TSC) &&
+		!WARN_ON_ONCE(!sev_snp_guest(kvm));
+}
 #else
 #define sev_guest(kvm) false
 #define sev_es_guest(kvm) false
 #define sev_snp_guest(kvm) false
+#define snp_secure_tsc_enabled(kvm) false
 #endif
 
 static inline bool ghcb_gpa_is_registered(struct vcpu_svm *svm, u64 val)
-- 
2.43.0

[PATCH v3 4/5] KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled

[Nikunj A Dadhania]

Disallow writes to MSR_IA32_TSC for Secure TSC enabled SNP guests. Even if
KVM attempts to emulate such writes, TSC calculation will ignore the
TSC_SCALE and TSC_OFFSET present in the VMCB. Instead, it will use
GUEST_TSC_SCALE and GUEST_TSC_OFFSET stored in the VMSA.

Additionally, incorporate a check for protected guest state to allow the
VMM to initialize the TSC MSR.

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

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 93cf508f983c..7463466f5126 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3161,6 +3161,20 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 
 		svm->tsc_aux = data;
 		break;
+	case MSR_IA32_TSC:
+		/*
+		 * If Secure TSC is enabled, do not emulate TSC write as TSC calculation
+		 * ignores the TSC_OFFSET and TSC_SCALE control fields, record the error
+		 * and return a #GP. Allow the TSC to be initialized until the guest state
+		 * is protected to prevent unexpected VMM errors.
+		 */
+		if (vcpu->arch.guest_state_protected && snp_secure_tsc_enabled(vcpu->kvm)) {
+			vcpu_unimpl(vcpu, "unimplemented IA32_TSC for secure tsc\n");
+			return 1;
+		}
+
+		ret = kvm_set_msr_common(vcpu, msr);
+		break;
 	case MSR_IA32_DEBUGCTLMSR:
 		if (!lbrv) {
 			kvm_pr_unimpl_wrmsr(vcpu, ecx, data);
-- 
2.43.0

[PATCH v3 5/5] KVM: SVM: Enable Secure TSC for SNP guests

[Nikunj A Dadhania]

From: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>

Add support for Secure TSC, allowing userspace to configure the Secure TSC
feature for SNP guests. Use the SNP specification's desired TSC frequency
parameter during the SNP_LAUNCH_START command to set the mean TSC
frequency in KHz for Secure TSC enabled guests. If the frequency is not
specified by the VMM, default to tsc_khz.

Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
Co-developed-by: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 arch/x86/include/uapi/asm/kvm.h |  3 ++-
 arch/x86/kvm/svm/sev.c          | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 9e75da97bce0..87ed9f77314d 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -836,7 +836,8 @@ struct kvm_sev_snp_launch_start {
 	__u64 policy;
 	__u8 gosvw[16];
 	__u16 flags;
-	__u8 pad0[6];
+	__u8 pad0[2];
+	__u32 desired_tsc_khz;
 	__u64 pad1[4];
 };
 
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 7875bb14a2b1..0b2112360844 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2207,6 +2207,20 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
 
 	start.gctx_paddr = __psp_pa(sev->snp_context);
 	start.policy = params.policy;
+
+	if (snp_secure_tsc_enabled(kvm)) {
+		u32 user_tsc_khz = params.desired_tsc_khz;
+
+		/* Use tsc_khz if the VMM has not provided the TSC frequency */
+		if (!user_tsc_khz)
+			user_tsc_khz = tsc_khz;
+
+		start.desired_tsc_khz = user_tsc_khz;
+
+		/* Set the arch default TSC for the VM*/
+		kvm->arch.default_tsc_khz = user_tsc_khz;
+	}
+
 	memcpy(start.gosvw, params.gosvw, sizeof(params.gosvw));
 	rc = __sev_issue_cmd(argp->sev_fd, SEV_CMD_SNP_LAUNCH_START, &start, &argp->error);
 	if (rc) {
@@ -2929,6 +2943,9 @@ void __init sev_set_cpu_caps(void)
 	if (sev_snp_enabled) {
 		kvm_cpu_cap_set(X86_FEATURE_SEV_SNP);
 		kvm_caps.supported_vm_types |= BIT(KVM_X86_SNP_VM);
+
+		if (cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
+			kvm_cpu_cap_set(X86_FEATURE_SNP_SECURE_TSC);
 	}
 }
 
@@ -3061,6 +3078,9 @@ void __init sev_hardware_setup(void)
 	sev_supported_vmsa_features = 0;
 	if (sev_es_debug_swap_enabled)
 		sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
+
+	if (sev_snp_enabled && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
+		sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
 }
 
 void sev_hardware_unsetup(void)
-- 
2.43.0

Re: [PATCH v3 3/5] KVM: SVM: Add GUEST_TSC_FREQ MSR for Secure TSC enabled guests

[Tom Lendacky]

On 2/17/25 04:22, Nikunj A Dadhania wrote:
> Introduce the read-only MSR GUEST_TSC_FREQ (0xc0010134) that returns
> guest's effective frequency in MHZ when Secure TSC is enabled for SNP
> guests. Disable interception of this MSR when Secure TSC is enabled. Note
> that GUEST_TSC_FREQ MSR is accessible only to the guest and not from the
> hypervisor context.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
>  arch/x86/include/asm/svm.h |  1 +
>  arch/x86/kvm/svm/sev.c     |  2 ++
>  arch/x86/kvm/svm/svm.c     |  1 +
>  arch/x86/kvm/svm/svm.h     | 11 ++++++++++-
>  4 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index e2fac21471f5..a04346068c60 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -289,6 +289,7 @@ static_assert((X2AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AVIC_
>  #define SVM_SEV_FEAT_RESTRICTED_INJECTION		BIT(3)
>  #define SVM_SEV_FEAT_ALTERNATE_INJECTION		BIT(4)
>  #define SVM_SEV_FEAT_DEBUG_SWAP				BIT(5)
> +#define SVM_SEV_FEAT_SECURE_TSC				BIT(9)
>  
>  #define SVM_SEV_FEAT_INT_INJ_MODES		\
>  	(SVM_SEV_FEAT_RESTRICTED_INJECTION |	\
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 74525651770a..7875bb14a2b1 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -843,6 +843,8 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
>  	save->dr6  = svm->vcpu.arch.dr6;
>  
>  	save->sev_features = sev->vmsa_features;
> +	if (snp_secure_tsc_enabled(vcpu->kvm))
> +		set_msr_interception(&svm->vcpu, svm->msrpm, MSR_AMD64_GUEST_TSC_FREQ, 1, 1);

Seems odd to clear the intercept in the sev_es_sync_vmsa() routine. Why
not in the sev_es_init_vmcb() routine where this is normally done?

Thanks,
Tom

>  
>  	/*
>  	 * Skip FPU and AVX setup with KVM_SEV_ES_INIT to avoid
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index b8aa0f36850f..93cf508f983c 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -143,6 +143,7 @@ static const struct svm_direct_access_msrs {
>  	{ .index = X2APIC_MSR(APIC_TMICT),		.always = false },
>  	{ .index = X2APIC_MSR(APIC_TMCCT),		.always = false },
>  	{ .index = X2APIC_MSR(APIC_TDCR),		.always = false },
> +	{ .index = MSR_AMD64_GUEST_TSC_FREQ,		.always = false },
>  	{ .index = MSR_INVALID,				.always = false },
>  };
>  
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 5b159f017055..7335af2ab1df 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -44,7 +44,7 @@ static inline struct page *__sme_pa_to_page(unsigned long pa)
>  #define	IOPM_SIZE PAGE_SIZE * 3
>  #define	MSRPM_SIZE PAGE_SIZE * 2
>  
> -#define MAX_DIRECT_ACCESS_MSRS	48
> +#define MAX_DIRECT_ACCESS_MSRS	49
>  #define MSRPM_OFFSETS	32
>  extern u32 msrpm_offsets[MSRPM_OFFSETS] __read_mostly;
>  extern bool npt_enabled;
> @@ -377,10 +377,19 @@ static __always_inline bool sev_snp_guest(struct kvm *kvm)
>  	return (sev->vmsa_features & SVM_SEV_FEAT_SNP_ACTIVE) &&
>  	       !WARN_ON_ONCE(!sev_es_guest(kvm));
>  }
> +
> +static inline bool snp_secure_tsc_enabled(struct kvm *kvm)
> +{
> +	struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
> +
> +	return (sev->vmsa_features & SVM_SEV_FEAT_SECURE_TSC) &&
> +		!WARN_ON_ONCE(!sev_snp_guest(kvm));
> +}
>  #else
>  #define sev_guest(kvm) false
>  #define sev_es_guest(kvm) false
>  #define sev_snp_guest(kvm) false
> +#define snp_secure_tsc_enabled(kvm) false
>  #endif
>  
>  static inline bool ghcb_gpa_is_registered(struct vcpu_svm *svm, u64 val)

Re: [PATCH v3 5/5] KVM: SVM: Enable Secure TSC for SNP guests

[Tom Lendacky]

On 2/17/25 04:22, Nikunj A Dadhania wrote:
> From: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
> 
> Add support for Secure TSC, allowing userspace to configure the Secure TSC
> feature for SNP guests. Use the SNP specification's desired TSC frequency
> parameter during the SNP_LAUNCH_START command to set the mean TSC
> frequency in KHz for Secure TSC enabled guests. If the frequency is not
> specified by the VMM, default to tsc_khz.
> 
> Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
> Co-developed-by: Nikunj A Dadhania <nikunj@amd.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
>  arch/x86/include/uapi/asm/kvm.h |  3 ++-
>  arch/x86/kvm/svm/sev.c          | 20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 9e75da97bce0..87ed9f77314d 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -836,7 +836,8 @@ struct kvm_sev_snp_launch_start {
>  	__u64 policy;
>  	__u8 gosvw[16];
>  	__u16 flags;
> -	__u8 pad0[6];
> +	__u8 pad0[2];
> +	__u32 desired_tsc_khz;
>  	__u64 pad1[4];
>  };
>  
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 7875bb14a2b1..0b2112360844 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2207,6 +2207,20 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  
>  	start.gctx_paddr = __psp_pa(sev->snp_context);
>  	start.policy = params.policy;
> +
> +	if (snp_secure_tsc_enabled(kvm)) {
> +		u32 user_tsc_khz = params.desired_tsc_khz;
> +
> +		/* Use tsc_khz if the VMM has not provided the TSC frequency */
> +		if (!user_tsc_khz)
> +			user_tsc_khz = tsc_khz;
> +
> +		start.desired_tsc_khz = user_tsc_khz;
> +
> +		/* Set the arch default TSC for the VM*/
> +		kvm->arch.default_tsc_khz = user_tsc_khz;
> +	}
> +
>  	memcpy(start.gosvw, params.gosvw, sizeof(params.gosvw));
>  	rc = __sev_issue_cmd(argp->sev_fd, SEV_CMD_SNP_LAUNCH_START, &start, &argp->error);
>  	if (rc) {
> @@ -2929,6 +2943,9 @@ void __init sev_set_cpu_caps(void)
>  	if (sev_snp_enabled) {
>  		kvm_cpu_cap_set(X86_FEATURE_SEV_SNP);
>  		kvm_caps.supported_vm_types |= BIT(KVM_X86_SNP_VM);
> +
> +		if (cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
> +			kvm_cpu_cap_set(X86_FEATURE_SNP_SECURE_TSC);

kvm_cpu_cap_check_and_set()

Thanks,
Tom

>  	}
>  }
>  
> @@ -3061,6 +3078,9 @@ void __init sev_hardware_setup(void)
>  	sev_supported_vmsa_features = 0;
>  	if (sev_es_debug_swap_enabled)
>  		sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
> +
> +	if (sev_snp_enabled && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
> +		sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
>  }
>  
>  void sev_hardware_unsetup(void)

Re: [PATCH v3 4/5] KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled

[Tom Lendacky]

On 2/17/25 04:22, Nikunj A Dadhania wrote:
> Disallow writes to MSR_IA32_TSC for Secure TSC enabled SNP guests. Even if
> KVM attempts to emulate such writes, TSC calculation will ignore the
> TSC_SCALE and TSC_OFFSET present in the VMCB. Instead, it will use
> GUEST_TSC_SCALE and GUEST_TSC_OFFSET stored in the VMSA.
> 
> Additionally, incorporate a check for protected guest state to allow the
> VMM to initialize the TSC MSR.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
>  arch/x86/kvm/svm/svm.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 93cf508f983c..7463466f5126 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3161,6 +3161,20 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  
>  		svm->tsc_aux = data;
>  		break;
> +	case MSR_IA32_TSC:
> +		/*
> +		 * If Secure TSC is enabled, do not emulate TSC write as TSC calculation
> +		 * ignores the TSC_OFFSET and TSC_SCALE control fields, record the error
> +		 * and return a #GP. Allow the TSC to be initialized until the guest state
> +		 * is protected to prevent unexpected VMM errors.
> +		 */
> +		if (vcpu->arch.guest_state_protected && snp_secure_tsc_enabled(vcpu->kvm)) {

I'm not sure if it matters, but do we need to differentiate between
guest and host write in this situation at all in regards to the message
or return code?

> +			vcpu_unimpl(vcpu, "unimplemented IA32_TSC for secure tsc\n");

s/secure tsc/Secure TSC/ ?

Thanks,
Tom

> +			return 1;
> +		}
> +
> +		ret = kvm_set_msr_common(vcpu, msr);
> +		break;
>  	case MSR_IA32_DEBUGCTLMSR:
>  		if (!lbrv) {
>  			kvm_pr_unimpl_wrmsr(vcpu, ecx, data);

Re: [PATCH v3 2/5] crypto: ccp: Add missing member in SNP_LAUNCH_START command structure

[Tom Lendacky]

On 2/17/25 04:22, Nikunj A Dadhania wrote:
> The sev_data_snp_launch_start structure should include a 4-byte
> desired_tsc_khz field before the gosvw field, which was missed in the
> initial implementation. As a result, the structure is 4 bytes shorter than
> expected by the firmware, causing the gosvw field to start 4 bytes early.
> Fix this by adding the missing 4-byte member for the desired TSC frequency.
> 
> Fixes: 3a45dc2b419e ("crypto: ccp: Define the SEV-SNP commands")
> Cc: stable@vger.kernel.org
> Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

Even though you're using the "crypto: ccp:" tag (which should actually
be "crypto: ccp -"), this can probably go through the KVM tree. Not sure
if it makes sense to tag it as "KVM: SVM:" instead.

Thanks,
Tom

> ---
>  include/linux/psp-sev.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index f3cad182d4ef..1f3620aaa4e7 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -594,6 +594,7 @@ struct sev_data_snp_addr {
>   * @imi_en: launch flow is launching an IMI (Incoming Migration Image) for the
>   *          purpose of guest-assisted migration.
>   * @rsvd: reserved
> + * @desired_tsc_khz: hypervisor desired mean TSC freq in kHz of the guest
>   * @gosvw: guest OS-visible workarounds, as defined by hypervisor
>   */
>  struct sev_data_snp_launch_start {
> @@ -603,6 +604,7 @@ struct sev_data_snp_launch_start {
>  	u32 ma_en:1;				/* In */
>  	u32 imi_en:1;				/* In */
>  	u32 rsvd:30;
> +	u32 desired_tsc_khz;			/* In */
>  	u8 gosvw[16];				/* In */
>  } __packed;
>  

Re: [PATCH v3 2/5] crypto: ccp: Add missing member in SNP_LAUNCH_START command structure

[Nikunj A Dadhania]

Tom Lendacky <thomas.lendacky@amd.com> writes:

> On 2/17/25 04:22, Nikunj A Dadhania wrote:
>> The sev_data_snp_launch_start structure should include a 4-byte
>> desired_tsc_khz field before the gosvw field, which was missed in the
>> initial implementation. As a result, the structure is 4 bytes shorter than
>> expected by the firmware, causing the gosvw field to start 4 bytes early.
>> Fix this by adding the missing 4-byte member for the desired TSC frequency.
>> 
>> Fixes: 3a45dc2b419e ("crypto: ccp: Define the SEV-SNP commands")
>> Cc: stable@vger.kernel.org
>> Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>
> Even though you're using the "crypto: ccp:" tag (which should actually
> be "crypto: ccp -"), this can probably go through the KVM tree. Not sure
> if it makes sense to tag it as "KVM: SVM:" instead.

Thanks, I will update the subject.

Regards
Nikunj

Re: [PATCH v3 3/5] KVM: SVM: Add GUEST_TSC_FREQ MSR for Secure TSC enabled guests

[Nikunj A Dadhania]

Tom Lendacky <thomas.lendacky@amd.com> writes:

> On 2/17/25 04:22, Nikunj A Dadhania wrote:
>> Introduce the read-only MSR GUEST_TSC_FREQ (0xc0010134) that returns
>> guest's effective frequency in MHZ when Secure TSC is enabled for SNP
>> guests. Disable interception of this MSR when Secure TSC is enabled. Note
>> that GUEST_TSC_FREQ MSR is accessible only to the guest and not from the
>> hypervisor context.
>> 
>> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
>> ---
>>  arch/x86/include/asm/svm.h |  1 +
>>  arch/x86/kvm/svm/sev.c     |  2 ++
>>  arch/x86/kvm/svm/svm.c     |  1 +
>>  arch/x86/kvm/svm/svm.h     | 11 ++++++++++-
>>  4 files changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
>> index e2fac21471f5..a04346068c60 100644
>> --- a/arch/x86/include/asm/svm.h
>> +++ b/arch/x86/include/asm/svm.h
>> @@ -289,6 +289,7 @@ static_assert((X2AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AVIC_
>>  #define SVM_SEV_FEAT_RESTRICTED_INJECTION		BIT(3)
>>  #define SVM_SEV_FEAT_ALTERNATE_INJECTION		BIT(4)
>>  #define SVM_SEV_FEAT_DEBUG_SWAP				BIT(5)
>> +#define SVM_SEV_FEAT_SECURE_TSC				BIT(9)
>>  
>>  #define SVM_SEV_FEAT_INT_INJ_MODES		\
>>  	(SVM_SEV_FEAT_RESTRICTED_INJECTION |	\
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index 74525651770a..7875bb14a2b1 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -843,6 +843,8 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
>>  	save->dr6  = svm->vcpu.arch.dr6;
>>  
>>  	save->sev_features = sev->vmsa_features;
>> +	if (snp_secure_tsc_enabled(vcpu->kvm))
>> +		set_msr_interception(&svm->vcpu, svm->msrpm, MSR_AMD64_GUEST_TSC_FREQ, 1, 1);
>
> Seems odd to clear the intercept in the sev_es_sync_vmsa() routine. Why
> not in the sev_es_init_vmcb() routine where this is normally done?

No particular reason that I can remember, I will move this to
sev_es_init_vmcb().

Regards,
Nikunj

Re: [PATCH v3 5/5] KVM: SVM: Enable Secure TSC for SNP guests

[Nikunj A Dadhania]

Tom Lendacky <thomas.lendacky@amd.com> writes:

> On 2/17/25 04:22, Nikunj A Dadhania wrote:
>> From: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>

>>  	if (rc) {
>> @@ -2929,6 +2943,9 @@ void __init sev_set_cpu_caps(void)
>>  	if (sev_snp_enabled) {
>>  		kvm_cpu_cap_set(X86_FEATURE_SEV_SNP);
>>  		kvm_caps.supported_vm_types |= BIT(KVM_X86_SNP_VM);
>> +
>> +		if (cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
>> +			kvm_cpu_cap_set(X86_FEATURE_SNP_SECURE_TSC);
>
> kvm_cpu_cap_check_and_set()

Sure, I have updated the patch.

Regards
Nikunj

Re: [PATCH v3 4/5] KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled

[Nikunj A Dadhania]

Tom Lendacky <thomas.lendacky@amd.com> writes:

> On 2/17/25 04:22, Nikunj A Dadhania wrote:

>> @@ -3161,6 +3161,20 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>>  
>>  		svm->tsc_aux = data;
>>  		break;
>> +	case MSR_IA32_TSC:
>> +		/*
>> +		 * If Secure TSC is enabled, do not emulate TSC write as TSC calculation
>> +		 * ignores the TSC_OFFSET and TSC_SCALE control fields, record the error
>> +		 * and return a #GP. Allow the TSC to be initialized until the guest state
>> +		 * is protected to prevent unexpected VMM errors.
>> +		 */
>> +		if (vcpu->arch.guest_state_protected && snp_secure_tsc_enabled(vcpu->kvm)) {
>
> I'm not sure if it matters, but do we need to differentiate between
> guest and host write in this situation at all in regards to the message
> or return code?
>

Yes, I think we can have something like the below:

+	case MSR_IA32_TSC:
+		/*
+		 * For Secure TSC enabled VM, do not emulate TSC write as the
+		 * TSC calculation ignores the TSC_OFFSET and TSC_SCALE control
+		 * fields.
+		 *
+		 * Guest writes: Record the error and return a #GP.
+		 * Host writes are ignored.
+		 */
+		if (snp_secure_tsc_enabled(vcpu->kvm)) {
+			if (!msr->host_initiated) {
+				vcpu_unimpl(vcpu, "unimplemented IA32_TSC for Secure TSC\n");
+				return 1;
+			} else
+				return 0;
+		}
+
+		ret = kvm_set_msr_common(vcpu, msr);
+		break;

>> +			vcpu_unimpl(vcpu, "unimplemented IA32_TSC for secure tsc\n");
>
> s/secure tsc/Secure TSC/ ?
>

Ack,

Thanks
Nikunj