[PATCH v6 0/4] Enable Secure TSC for SEV-SNP

[PATCH v6 0/4] 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/master

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:
----------
v6:
* Rebased on top of kvm/master
* Collected Reviewed-by/Tested-by
* s/svm->vcpu/vcpu/ in snp_launch_update_vmsa() as vcpu pointer is already available (Tom)
* Simplify assignment of guest_protected_tsc (Tom)


v5: https://lore.kernel.org/kvm/20250317052308.498244-1-nikunj@amd.com/
* Rebased on top of kvm/queue that includes protected TSC patches
  https://lore.kernel.org/kvm/20250314183422.2990277-1-pbonzini@redhat.com/
* Dropped patch 4/5 as it is not required after protected TSC patches
* Set guest_tsc_protected when Secure TSC is enabled (Paolo)
* Collect Reviewed-by from Tom
* Base the desired_tsc_freq on KVM's ABI (Sean)

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

Nikunj A Dadhania (3):
  x86/cpufeatures: Add SNP Secure TSC
  KVM: SVM: Add missing member in SNP_LAUNCH_START command structure
  KVM: SVM: Add GUEST_TSC_FREQ MSR for Secure TSC enabled guests

 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             | 18 +++++++++++++++++-
 arch/x86/kvm/svm/svm.c             |  1 +
 arch/x86/kvm/svm/svm.h             | 11 ++++++++++-
 include/linux/psp-sev.h            |  2 ++
 7 files changed, 34 insertions(+), 3 deletions(-)


base-commit: c77eee50caa289fee6cfde146471aa7b0f311471
-- 
2.43.0

[PATCH v6 1/4] 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".

Acked-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Tested-by: Vaishali Thakkar <vaishali.thakkar@suse.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 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 8f8aaf94dc00..68a4d6b4cc11 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -449,6 +449,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 v6 2/4] KVM: SVM: 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>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Tested-by: Vaishali Thakkar <vaishali.thakkar@suse.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 v6 3/4] 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.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Tested-by: Vaishali Thakkar <vaishali.thakkar@suse.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 arch/x86/include/asm/svm.h |  1 +
 arch/x86/kvm/svm/sev.c     |  3 +++
 arch/x86/kvm/svm/svm.c     |  1 +
 arch/x86/kvm/svm/svm.h     | 11 ++++++++++-
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 9b7fa99ae951..6ab66b80e751 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -290,6 +290,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)
 
 struct vmcb_seg {
 	u16 selector;
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 0bc708ee2788..50263b473f95 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4504,6 +4504,9 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
 	/* Clear intercepts on selected MSRs */
 	set_msr_interception(vcpu, svm->msrpm, MSR_EFER, 1, 1);
 	set_msr_interception(vcpu, svm->msrpm, MSR_IA32_CR_PAT, 1, 1);
+
+	if (snp_secure_tsc_enabled(vcpu->kvm))
+		set_msr_interception(vcpu, svm->msrpm, MSR_AMD64_GUEST_TSC_FREQ, 1, 1);
 }
 
 void sev_init_vmcb(struct vcpu_svm *svm)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 8abeab91d329..e65721db1f81 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 d4490eaed55d..711e21b7a3d0 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 v6 4/4] 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.

As the frequency needs to be set in the SNP_LAUNCH_START command, userspace
should set the frequency using the KVM_CAP_SET_TSC_KHZ VM ioctl instead of
the VCPU ioctl. The desired_tsc_khz defaults to kvm->arch.default_tsc_khz.

Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
Co-developed-by: Nikunj A Dadhania <nikunj@amd.com>
Tested-by: Vaishali Thakkar <vaishali.thakkar@suse.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@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          | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 460306b35a4b..075af0dcee25 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -839,7 +839,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 50263b473f95..bcb262ff42bb 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2205,6 +2205,14 @@ 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)) {
+		if (!kvm->arch.default_tsc_khz)
+			return -EINVAL;
+
+		start.desired_tsc_khz = kvm->arch.default_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) {
@@ -2445,7 +2453,9 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
 			return ret;
 		}
 
-		svm->vcpu.arch.guest_state_protected = true;
+		vcpu->arch.guest_state_protected = true;
+		vcpu->arch.guest_tsc_protected = snp_secure_tsc_enabled(kvm);
+
 		/*
 		 * SEV-ES (and thus SNP) guest mandates LBR Virtualization to
 		 * be _always_ ON. Enable it only after setting
@@ -3059,6 +3069,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 v6 0/4] Enable Secure TSC for SEV-SNP

[Nikunj A Dadhania]

On 4/8/2025 3:02 PM, Nikunj A Dadhania wrote:
> 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/master
> 
> 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:
> ----------
> v6:
> * Rebased on top of kvm/master
> * Collected Reviewed-by/Tested-by
> * s/svm->vcpu/vcpu/ in snp_launch_update_vmsa() as vcpu pointer is already available (Tom)
> * Simplify assignment of guest_protected_tsc (Tom)

A gentle reminder, any other suggestions/improvement ?

Regards
Nikunj

Re: [PATCH v6 0/4] Enable Secure TSC for SEV-SNP

[Nikunj A. Dadhania]

On 4/14/2025 11:20 AM, Nikunj A Dadhania wrote:
> On 4/8/2025 3:02 PM, Nikunj A Dadhania wrote:
>> 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/master
>>
>> 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:
>> ----------
>> v6:
>> * Rebased on top of kvm/master
>> * Collected Reviewed-by/Tested-by
>> * s/svm->vcpu/vcpu/ in snp_launch_update_vmsa() as vcpu pointer is already available (Tom)
>> * Simplify assignment of guest_protected_tsc (Tom)
> 
> A gentle reminder, any other suggestions/improvement ?
> 

A gentle reminder.

Regards
Nikunj

Re: [PATCH v6 0/4] Enable Secure TSC for SEV-SNP

[Nikunj A. Dadhania]

On 5/5/2025 10:20 AM, Nikunj A. Dadhania wrote:
> On 4/14/2025 11:20 AM, Nikunj A Dadhania wrote:
>> On 4/8/2025 3:02 PM, Nikunj A Dadhania wrote:
>>> Changelog:
>>> ----------
>>> v6:
>>> * Rebased on top of kvm/master
>>> * Collected Reviewed-by/Tested-by
>>> * s/svm->vcpu/vcpu/ in snp_launch_update_vmsa() as vcpu pointer is already available (Tom)
>>> * Simplify assignment of guest_protected_tsc (Tom)
>>
>> A gentle reminder, any other suggestions/improvement ?
>>
> 
> A gentle reminder.
>

A gentle reminder.

Regards,
Nikunj

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

[Sean Christopherson]

On Tue, Apr 08, 2025, 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.
> 
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Tested-by: Vaishali Thakkar <vaishali.thakkar@suse.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
>  arch/x86/include/asm/svm.h |  1 +
>  arch/x86/kvm/svm/sev.c     |  3 +++
>  arch/x86/kvm/svm/svm.c     |  1 +
>  arch/x86/kvm/svm/svm.h     | 11 ++++++++++-
>  4 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 9b7fa99ae951..6ab66b80e751 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -290,6 +290,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)
>  
>  struct vmcb_seg {
>  	u16 selector;
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 0bc708ee2788..50263b473f95 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -4504,6 +4504,9 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
>  	/* Clear intercepts on selected MSRs */
>  	set_msr_interception(vcpu, svm->msrpm, MSR_EFER, 1, 1);
>  	set_msr_interception(vcpu, svm->msrpm, MSR_IA32_CR_PAT, 1, 1);
> +
> +	if (snp_secure_tsc_enabled(vcpu->kvm))
> +		set_msr_interception(vcpu, svm->msrpm, MSR_AMD64_GUEST_TSC_FREQ, 1, 1);
>  }
>  
>  void sev_init_vmcb(struct vcpu_svm *svm)
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 8abeab91d329..e65721db1f81 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 d4490eaed55d..711e21b7a3d0 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)

This is only ever used in sev.c, it has no business living in svm.c.  And there's
especially no reason to have a stub for CONFIG_KVM_AMD_SEV=n.
> +{
> +	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
> 

Re: [PATCH v6 4/4] KVM: SVM: Enable Secure TSC for SNP guests

[Sean Christopherson]

The previous patch to add GUEST_TSC_FREQ needs to squashed with this patch.  It's
impossible to review the snp_secure_tsc_enabled() logic in particular without the
details added in this patch.

And once you rebase on kvm-x86 next (i.e. the MSR interception rework), adding
support for GUEST_TSC_FREQ will be like three lines of code, i.e. not worth
landing in a separate patch.

On Tue, Apr 08, 2025, 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.
> 
> As the frequency needs to be set in the SNP_LAUNCH_START command, userspace
> should set the frequency using the KVM_CAP_SET_TSC_KHZ VM ioctl instead of
> the VCPU ioctl. The desired_tsc_khz defaults to kvm->arch.default_tsc_khz.
> 
> Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
> Co-developed-by: Nikunj A Dadhania <nikunj@amd.com>
> Tested-by: Vaishali Thakkar <vaishali.thakkar@suse.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@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          | 15 ++++++++++++++-
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 460306b35a4b..075af0dcee25 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -839,7 +839,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 50263b473f95..bcb262ff42bb 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2205,6 +2205,14 @@ 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)) {
> +		if (!kvm->arch.default_tsc_khz)

Hmm, so there's an existing flaw related to the TSC frequency.  Ideally, KVM
shouldn't allow KVM_SET_TSC_KHZ on a vCPU with a "secure" TSC, i.e. on a TDX
vCPU or on a newfangled SNP vCPU.  I'm not sure that's worth addressing though,
because it doesn't put KVM in any danger, it can only cause problems for guest
timing.  Yeah, I guess we leave it, because it's not really any different than
enumerating a TSC frequency in CPUID 0x15 and then telling KVM something
different.

> +			return -EINVAL;
> +
> +		start.desired_tsc_khz = kvm->arch.default_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) {
> @@ -2445,7 +2453,9 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  			return ret;
>  		}
>  
> -		svm->vcpu.arch.guest_state_protected = true;
> +		vcpu->arch.guest_state_protected = true;
> +		vcpu->arch.guest_tsc_protected = snp_secure_tsc_enabled(kvm);
> +
>  		/*
>  		 * SEV-ES (and thus SNP) guest mandates LBR Virtualization to
>  		 * be _always_ ON. Enable it only after setting
> @@ -3059,6 +3069,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;

I don't see anything in here that prevents userspace from stuffing SECURE_TSC
into vmsa_features, which means the WARN_ON_ONCE() in snp_secure_tsc_enabled is
user-triggerable.

Unless I'm missing something, this need to do something like:

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 45283a2d8c4a..09044f2524c2 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -405,9 +405,13 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
        struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
        struct sev_platform_init_args init_args = {0};
        bool es_active = vm_type != KVM_X86_SEV_VM;
+       bool snp_active = vm_type -= KVM_X86_SNP_VM;
        u64 valid_vmsa_features = es_active ? sev_supported_vmsa_features : 0;
        int ret;
 
+       if (!snp_active)
+               valid_vmsa_features &= ~SVM_SEV_FEAT_SECURE_TSC;
+
        if (kvm->created_vcpus)
                return -EINVAL;
 
@@ -436,7 +440,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
        if (sev->es_active && !sev->ghcb_version)
                sev->ghcb_version = GHCB_VERSION_DEFAULT;
 
-       if (vm_type == KVM_X86_SNP_VM)
+       if (snp_active)
                sev->vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE;
 
        ret = sev_asid_new(sev);
@@ -449,7 +453,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
                goto e_free;
 
        /* This needs to happen after SEV/SNP firmware initialization. */
-       if (vm_type == KVM_X86_SNP_VM) {
+       if (snp_active) {
                ret = snp_guest_req_init(kvm);
                if (ret)
                        goto e_free;

Re: [PATCH v6 0/4] Enable Secure TSC for SEV-SNP

[Sean Christopherson]

On Tue, 08 Apr 2025 15:02:09 +0530, Nikunj A Dadhania wrote:
> 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.
> 
> [...]

Applied patch 2 to kvm-x86 fixes.  I'll wait for v7 to grab the others (and
stating the obvious, not for 6.16).

[2/4] KVM: SVM: Add missing member in SNP_LAUNCH_START command structure
      https://github.com/kvm-x86/linux/commit/51a4273dcab3

--
https://github.com/kvm-x86/kvm-unit-tests/tree/next

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

[Nikunj A. Dadhania]

On 6/25/2025 7:43 PM, Sean Christopherson wrote:
> On Tue, Apr 08, 2025, Nikunj A Dadhania wrote:
>> @@ -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)
> 
> This is only ever used in sev.c, it has no business living in svm.c.  And there's
> especially no reason to have a stub for CONFIG_KVM_AMD_SEV=n.

Ack, will move this to sev.c and remove the stub as well.

Regards
Nikunj

Re: [PATCH v6 0/4] Enable Secure TSC for SEV-SNP

[Nikunj A. Dadhania]

On 6/26/2025 3:55 AM, Sean Christopherson wrote:
> On Tue, 08 Apr 2025 15:02:09 +0530, Nikunj A Dadhania wrote:
>> 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.
>>
>> [...]
> 
> Applied patch 2 to kvm-x86 fixes.  

Thanks Sean.

> I'll wait for v7 to grab the others (and stating the obvious, not for 6.16).

Sure will send a v7 on top of kvm-x86 changes along with the suggested changes.

> [2/4] KVM: SVM: Add missing member in SNP_LAUNCH_START command structure
>       https://github.com/kvm-x86/linux/commit/51a4273dcab3

Regards
Nikunj

Re: [PATCH v6 4/4] KVM: SVM: Enable Secure TSC for SNP guests

[Nikunj A. Dadhania]

On 6/25/2025 7:57 PM, Sean Christopherson wrote:
> The previous patch to add GUEST_TSC_FREQ needs to squashed with this patch.  It's
> impossible to review the snp_secure_tsc_enabled() logic in particular without the
> details added in this patch.
> 
> And once you rebase on kvm-x86 next (i.e. the MSR interception rework), adding
> support for GUEST_TSC_FREQ will be like three lines of code, i.e. not worth
> landing in a separate patch.

Ack.

>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index 50263b473f95..bcb262ff42bb 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -2205,6 +2205,14 @@ 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)) {
>> +		if (!kvm->arch.default_tsc_khz)
> 
> Hmm, so there's an existing flaw related to the TSC frequency.  Ideally, KVM
> shouldn't allow KVM_SET_TSC_KHZ on a vCPU with a "secure" TSC, i.e. on a TDX
> vCPU or on a newfangled SNP vCPU.  I'm not sure that's worth addressing though,
> because it doesn't put KVM in any danger, it can only cause problems for guest
> timing.  Yeah, I guess we leave it, because it's not really any different than
> enumerating a TSC frequency in CPUID 0x15 and then telling KVM something
> different.

We can prevent the host from setting KVM_SET_TSC_KHZ when arch.guest_tsc_protected
is set in the vCPU IOCTL. Although, doing that in VM IOCTL will be tricky and may
require to add something like kvm->arch.has_tsc_protected.

> 
>> +			return -EINVAL;
>> +
>> +		start.desired_tsc_khz = kvm->arch.default_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) {
>> @@ -2445,7 +2453,9 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
>>  			return ret;
>>  		}
>>  
>> -		svm->vcpu.arch.guest_state_protected = true;
>> +		vcpu->arch.guest_state_protected = true;
>> +		vcpu->arch.guest_tsc_protected = snp_secure_tsc_enabled(kvm);
>> +
>>  		/*
>>  		 * SEV-ES (and thus SNP) guest mandates LBR Virtualization to
>>  		 * be _always_ ON. Enable it only after setting
>> @@ -3059,6 +3069,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;
> 
> I don't see anything in here that prevents userspace from stuffing SECURE_TSC
> into vmsa_features, which means the WARN_ON_ONCE() in snp_secure_tsc_enabled is
> user-triggerable.

Right, my QEMU patches enable vmsa_features only for the sev-snp-guest object.
But you are right, vmsa_features is part of KVM_SEV_INIT2 and can enable
SECURE_TSC causing the WARN_ON_ONCE()

> 
> Unless I'm missing something, this need to do something like:
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 45283a2d8c4a..09044f2524c2 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -405,9 +405,13 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
>         struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
>         struct sev_platform_init_args init_args = {0};
>         bool es_active = vm_type != KVM_X86_SEV_VM;
> +       bool snp_active = vm_type -= KVM_X86_SNP_VM;

vm_type == KVM_X86_SNP_VM

>         u64 valid_vmsa_features = es_active ? sev_supported_vmsa_features : 0;
>         int ret;
>  
> +       if (!snp_active)
> +               valid_vmsa_features &= ~SVM_SEV_FEAT_SECURE_TSC;
> +
>         if (kvm->created_vcpus)
>                 return -EINVAL;
>  
> @@ -436,7 +440,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
>         if (sev->es_active && !sev->ghcb_version)
>                 sev->ghcb_version = GHCB_VERSION_DEFAULT;
>  
> -       if (vm_type == KVM_X86_SNP_VM)
> +       if (snp_active)
>                 sev->vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE;
>  
>         ret = sev_asid_new(sev);
> @@ -449,7 +453,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
>                 goto e_free;
>  
>         /* This needs to happen after SEV/SNP firmware initialization. */
> -       if (vm_type == KVM_X86_SNP_VM) {
> +       if (snp_active) {
>                 ret = snp_guest_req_init(kvm);
>                 if (ret)
>                         goto e_free;

I will fold this in.

Regards
Nikunj

1: https://lore.kernel.org/kvm/20250310064522.14100-3-nikunj@amd.com/