* [PATCH v4 3/5] KVM: SVM: Add GUEST_TSC_FREQ MSR for Secure TSC enabled guests
2025-03-10 6:45 ` [PATCH v4 2/5] KVM: SVM: Add missing member in SNP_LAUNCH_START command structure Nikunj A Dadhania
@ 2025-03-10 6:45 ` Nikunj A Dadhania
2025-03-10 15:09 ` Tom Lendacky
2025-03-10 6:45 ` [PATCH v4 4/5] KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled Nikunj A Dadhania
2025-03-10 6:45 ` [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests Nikunj A Dadhania
2 siblings, 1 reply; 20+ messages in thread
From: Nikunj A Dadhania @ 2025-03-10 6:45 UTC (permalink / raw)
To: seanjc, pbonzini, kvm
Cc: thomas.lendacky, santosh.shukla, bp, nikunj, isaku.yamahata
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 | 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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 3/5] KVM: SVM: Add GUEST_TSC_FREQ MSR for Secure TSC enabled guests
2025-03-10 6:45 ` [PATCH v4 3/5] KVM: SVM: Add GUEST_TSC_FREQ MSR for Secure TSC enabled guests Nikunj A Dadhania
@ 2025-03-10 15:09 ` Tom Lendacky
0 siblings, 0 replies; 20+ messages in thread
From: Tom Lendacky @ 2025-03-10 15:09 UTC (permalink / raw)
To: Nikunj A Dadhania, seanjc, pbonzini, kvm
Cc: santosh.shukla, bp, isaku.yamahata
On 3/10/25 01:45, 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>
Reviewed-by: Tom Lendacky <thomas.lendacky@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)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 4/5] KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled
2025-03-10 6:45 ` [PATCH v4 2/5] KVM: SVM: Add missing member in SNP_LAUNCH_START command structure Nikunj A Dadhania
2025-03-10 6:45 ` [PATCH v4 3/5] KVM: SVM: Add GUEST_TSC_FREQ MSR for Secure TSC enabled guests Nikunj A Dadhania
@ 2025-03-10 6:45 ` Nikunj A Dadhania
2025-03-10 15:23 ` Tom Lendacky
2025-03-10 6:45 ` [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests Nikunj A Dadhania
2 siblings, 1 reply; 20+ messages in thread
From: Nikunj A Dadhania @ 2025-03-10 6:45 UTC (permalink / raw)
To: seanjc, pbonzini, kvm
Cc: thomas.lendacky, santosh.shukla, bp, nikunj, isaku.yamahata
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 | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e65721db1f81..1652848b0240 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3161,6 +3161,25 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
svm->tsc_aux = data;
break;
+ 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;
case MSR_IA32_DEBUGCTLMSR:
if (!lbrv) {
kvm_pr_unimpl_wrmsr(vcpu, ecx, data);
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 4/5] KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled
2025-03-10 6:45 ` [PATCH v4 4/5] KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled Nikunj A Dadhania
@ 2025-03-10 15:23 ` Tom Lendacky
2025-03-11 6:49 ` Nikunj A. Dadhania
0 siblings, 1 reply; 20+ messages in thread
From: Tom Lendacky @ 2025-03-10 15:23 UTC (permalink / raw)
To: Nikunj A Dadhania, seanjc, pbonzini, kvm
Cc: santosh.shukla, bp, isaku.yamahata
On 3/10/25 01:45, 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
s/TSC calculation/the TSC calculation performed by hardware/
> TSC_SCALE and TSC_OFFSET present in the VMCB. Instead, it will use
s/present/values present/
s/VMCB. Instead, it will use/VMCB and instead use the/
> GUEST_TSC_SCALE and GUEST_TSC_OFFSET stored in the VMSA.
s/stored/values stored/
>
> Additionally, incorporate a check for protected guest state to allow the
> VMM to initialize the TSC MSR.
I don't see this in the patch.
>
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
With cleanup to the commit message and a formatting nit below:
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> arch/x86/kvm/svm/svm.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index e65721db1f81..1652848b0240 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3161,6 +3161,25 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>
> svm->tsc_aux = data;
> break;
> + 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;
You need "{" and "}" around this.
Thanks,
Tom
> + }
> +
> + ret = kvm_set_msr_common(vcpu, msr);
> + break;
> case MSR_IA32_DEBUGCTLMSR:
> if (!lbrv) {
> kvm_pr_unimpl_wrmsr(vcpu, ecx, data);
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 4/5] KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled
2025-03-10 15:23 ` Tom Lendacky
@ 2025-03-11 6:49 ` Nikunj A. Dadhania
0 siblings, 0 replies; 20+ messages in thread
From: Nikunj A. Dadhania @ 2025-03-11 6:49 UTC (permalink / raw)
To: Tom Lendacky, seanjc, pbonzini, kvm; +Cc: santosh.shukla, bp, isaku.yamahata
On 3/10/2025 8:53 PM, Tom Lendacky wrote:
> On 3/10/25 01:45, 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
>
> s/TSC calculation/the TSC calculation performed by hardware/
>
>> TSC_SCALE and TSC_OFFSET present in the VMCB. Instead, it will use
>
> s/present/values present/
> s/VMCB. Instead, it will use/VMCB and instead use the/
>> GUEST_TSC_SCALE and GUEST_TSC_OFFSET stored in the VMSA.
>
> s/stored/values stored/
>
Ack
>>
>> Additionally, incorporate a check for protected guest state to allow the
>> VMM to initialize the TSC MSR.
>
> I don't see this in the patch.
This change is removed as we are differentiating between host/guest writes
and host writes are ignored. I will remove this.
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
>
> With cleanup to the commit message and a formatting nit below:
>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>
>> ---
>> arch/x86/kvm/svm/svm.c | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index e65721db1f81..1652848b0240 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -3161,6 +3161,25 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>>
>> svm->tsc_aux = data;
>> break;
>> + 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;
>
> You need "{" and "}" around this.
Ack
Regards
Nikunj
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests
2025-03-10 6:45 ` [PATCH v4 2/5] KVM: SVM: Add missing member in SNP_LAUNCH_START command structure Nikunj A Dadhania
2025-03-10 6:45 ` [PATCH v4 3/5] KVM: SVM: Add GUEST_TSC_FREQ MSR for Secure TSC enabled guests Nikunj A Dadhania
2025-03-10 6:45 ` [PATCH v4 4/5] KVM: SVM: Prevent writes to TSC MSR when Secure TSC is enabled Nikunj A Dadhania
@ 2025-03-10 6:45 ` Nikunj A Dadhania
2025-03-10 15:39 ` Tom Lendacky
2 siblings, 1 reply; 20+ messages in thread
From: Nikunj A Dadhania @ 2025-03-10 6:45 UTC (permalink / raw)
To: seanjc, pbonzini, kvm
Cc: thomas.lendacky, santosh.shukla, bp, nikunj, isaku.yamahata
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 | 19 +++++++++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
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..b61d6bd75b37 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2205,6 +2205,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) {
@@ -2927,6 +2941,8 @@ 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);
+
+ kvm_cpu_cap_check_and_set(X86_FEATURE_SNP_SECURE_TSC);
}
}
@@ -3059,6 +3075,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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests
2025-03-10 6:45 ` [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests Nikunj A Dadhania
@ 2025-03-10 15:39 ` Tom Lendacky
2025-03-11 9:11 ` Nikunj A. Dadhania
0 siblings, 1 reply; 20+ messages in thread
From: Tom Lendacky @ 2025-03-10 15:39 UTC (permalink / raw)
To: Nikunj A Dadhania, seanjc, pbonzini, kvm
Cc: santosh.shukla, bp, isaku.yamahata
On 3/10/25 01:45, 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 | 19 +++++++++++++++++++
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> 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..b61d6bd75b37 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2205,6 +2205,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;
Do we need to perform any sanity checking against this value?
Thanks,
Tom
> +
> + /* 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) {
> @@ -2927,6 +2941,8 @@ 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);
> +
> + kvm_cpu_cap_check_and_set(X86_FEATURE_SNP_SECURE_TSC);
> }
> }
>
> @@ -3059,6 +3075,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)
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests
2025-03-10 15:39 ` Tom Lendacky
@ 2025-03-11 9:11 ` Nikunj A. Dadhania
2025-03-11 11:05 ` Nikunj A. Dadhania
0 siblings, 1 reply; 20+ messages in thread
From: Nikunj A. Dadhania @ 2025-03-11 9:11 UTC (permalink / raw)
To: Tom Lendacky, seanjc, pbonzini, kvm; +Cc: santosh.shukla, bp, isaku.yamahata
On 3/10/2025 9:09 PM, Tom Lendacky wrote:
> On 3/10/25 01:45, Nikunj A Dadhania wrote:
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index 50263b473f95..b61d6bd75b37 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -2205,6 +2205,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;
>
> Do we need to perform any sanity checking against this value?
On the higher side, sev-snp-guest.stsc-freq is u32, a Secure TSC guest boots fine with
TSC frequency set to the highest value (stsc-freq=0xFFFFFFFF).
On the lower side as MSR_AMD64_GUEST_TSC_FREQ is in MHz, TSC clock should at least be 1Mhz.
Any values below would either triggers a splat or crashes the guest kernel
For stsc-freq=1000 (1Khz), guest crash with the below:
kvm-clock: using sched offset of 4782335885 cycles
CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.14.0-rc5-00537-gcecc16fa7fac #254
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
RIP: 0010:pvclock_tsc_khz+0x13/0x40
For stsc-freq=500000 (500KHz), boots but with the below warning: basically tsc_khz is
zero as we are reading zero from MSR_AMD64_GUEST_TSC_FREQ.
WARNING: CPU: 0 PID: 0 at arch/x86/kernel/tsc.c:1463 determine_cpu_tsc_frequencies+0x11b/0x120
Regards
Nikunj
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests
2025-03-11 9:11 ` Nikunj A. Dadhania
@ 2025-03-11 11:05 ` Nikunj A. Dadhania
2025-03-11 14:33 ` Tom Lendacky
0 siblings, 1 reply; 20+ messages in thread
From: Nikunj A. Dadhania @ 2025-03-11 11:05 UTC (permalink / raw)
To: Tom Lendacky, seanjc, pbonzini, kvm; +Cc: santosh.shukla, bp, isaku.yamahata
On 3/11/2025 2:41 PM, Nikunj A. Dadhania wrote:
>
>
> On 3/10/2025 9:09 PM, Tom Lendacky wrote:
>> On 3/10/25 01:45, Nikunj A Dadhania wrote:
>
>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>>> index 50263b473f95..b61d6bd75b37 100644
>>> --- a/arch/x86/kvm/svm/sev.c
>>> +++ b/arch/x86/kvm/svm/sev.c
>>> @@ -2205,6 +2205,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;
>>
>> Do we need to perform any sanity checking against this value?
>
> On the higher side, sev-snp-guest.stsc-freq is u32, a Secure TSC guest boots fine with
> TSC frequency set to the highest value (stsc-freq=0xFFFFFFFF).
>
> On the lower side as MSR_AMD64_GUEST_TSC_FREQ is in MHz, TSC clock should at least be 1Mhz.
Something like this ?
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b61d6bd75b37..c46b6afa969d 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2213,6 +2213,14 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
if (!user_tsc_khz)
user_tsc_khz = tsc_khz;
+ /*
+ * The minimum granularity for reporting Secure TSC frequency is
+ * 1MHz. Return an error if the user specifies a TSC frequency
+ * less than 1MHz.
+ */
+ if (user_tsc_khz < 1000)
+ return -EINVAL;
+
start.desired_tsc_khz = user_tsc_khz;
/* Set the arch default TSC for the VM*/
Regards
Nikunj
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests
2025-03-11 11:05 ` Nikunj A. Dadhania
@ 2025-03-11 14:33 ` Tom Lendacky
2025-03-11 14:52 ` Sean Christopherson
0 siblings, 1 reply; 20+ messages in thread
From: Tom Lendacky @ 2025-03-11 14:33 UTC (permalink / raw)
To: Nikunj A. Dadhania, seanjc, pbonzini, kvm
Cc: santosh.shukla, bp, isaku.yamahata
On 3/11/25 06:05, Nikunj A. Dadhania wrote:
> On 3/11/2025 2:41 PM, Nikunj A. Dadhania wrote:
>> On 3/10/2025 9:09 PM, Tom Lendacky wrote:
>>> On 3/10/25 01:45, Nikunj A Dadhania wrote:
>>
>>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>>>> index 50263b473f95..b61d6bd75b37 100644
>>>> --- a/arch/x86/kvm/svm/sev.c
>>>> +++ b/arch/x86/kvm/svm/sev.c
>>>> @@ -2205,6 +2205,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;
>>>
>>> Do we need to perform any sanity checking against this value?
>>
>> On the higher side, sev-snp-guest.stsc-freq is u32, a Secure TSC guest boots fine with
>> TSC frequency set to the highest value (stsc-freq=0xFFFFFFFF).
>>
>> On the lower side as MSR_AMD64_GUEST_TSC_FREQ is in MHz, TSC clock should at least be 1Mhz.
>
> Something like this ?
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b61d6bd75b37..c46b6afa969d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2213,6 +2213,14 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> if (!user_tsc_khz)
> user_tsc_khz = tsc_khz;
>
> + /*
> + * The minimum granularity for reporting Secure TSC frequency is
> + * 1MHz. Return an error if the user specifies a TSC frequency
> + * less than 1MHz.
> + */
> + if (user_tsc_khz < 1000)
> + return -EINVAL;
Seems reasonable to me. I'll let Sean or Paolo weigh in on it. I don't
think we need a message, there should be a check in the VMM, too, which
would be able to provide information to the end user?
Thanks,
Tom
> +
> start.desired_tsc_khz = user_tsc_khz;
>
> /* Set the arch default TSC for the VM*/
>
> Regards
> Nikunj
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests
2025-03-11 14:33 ` Tom Lendacky
@ 2025-03-11 14:52 ` Sean Christopherson
2025-03-11 15:33 ` Nikunj A. Dadhania
0 siblings, 1 reply; 20+ messages in thread
From: Sean Christopherson @ 2025-03-11 14:52 UTC (permalink / raw)
To: Tom Lendacky
Cc: Nikunj A. Dadhania, pbonzini, kvm, santosh.shukla, bp,
isaku.yamahata
On Tue, Mar 11, 2025, Tom Lendacky wrote:
> On 3/11/25 06:05, Nikunj A. Dadhania wrote:
> > On 3/11/2025 2:41 PM, Nikunj A. Dadhania wrote:
> >> On 3/10/2025 9:09 PM, Tom Lendacky wrote:
> >>> On 3/10/25 01:45, Nikunj A Dadhania wrote:
> >>
> >>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> >>>> index 50263b473f95..b61d6bd75b37 100644
> >>>> --- a/arch/x86/kvm/svm/sev.c
> >>>> +++ b/arch/x86/kvm/svm/sev.c
> >>>> @@ -2205,6 +2205,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;
The code just below this clobbers kvm->arch.default_tsc_khz, which could already
have been set by userspace. Why? Either require params.desired_tsc_khz to match
kvm->arch.default_tsc_khz, or have KVM's ABI be that KVM stuffs desired_tsc_khz
based on kvm->arch.default_tsc_khz. I don't see any reason to add yet another
way to control TSC.
> >>> Do we need to perform any sanity checking against this value?
> >>
> >> On the higher side, sev-snp-guest.stsc-freq is u32, a Secure TSC guest boots fine with
> >> TSC frequency set to the highest value (stsc-freq=0xFFFFFFFF).
> >>
> >> On the lower side as MSR_AMD64_GUEST_TSC_FREQ is in MHz, TSC clock should at least be 1Mhz.
> >
> > Something like this ?
> >
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index b61d6bd75b37..c46b6afa969d 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -2213,6 +2213,14 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > if (!user_tsc_khz)
> > user_tsc_khz = tsc_khz;
> >
> > + /*
> > + * The minimum granularity for reporting Secure TSC frequency is
> > + * 1MHz. Return an error if the user specifies a TSC frequency
> > + * less than 1MHz.
> > + */
> > + if (user_tsc_khz < 1000)
> > + return -EINVAL;
>
> Seems reasonable to me. I'll let Sean or Paolo weigh in on it. I don't
> think we need a message, there should be a check in the VMM, too, which
> would be able to provide information to the end user?
Why bother? Userspace can DoS the guest anytime it wants. A TSC frequency of
1MHz on a modern CPU is absolutely absurd. Making that the minimum is is likely
going to do nothing but sow confusion.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests
2025-03-11 14:52 ` Sean Christopherson
@ 2025-03-11 15:33 ` Nikunj A. Dadhania
2025-03-11 15:53 ` Sean Christopherson
0 siblings, 1 reply; 20+ messages in thread
From: Nikunj A. Dadhania @ 2025-03-11 15:33 UTC (permalink / raw)
To: Sean Christopherson, Tom Lendacky
Cc: pbonzini, kvm, santosh.shukla, bp, isaku.yamahata
On 3/11/2025 8:22 PM, Sean Christopherson wrote:
> On Tue, Mar 11, 2025, Tom Lendacky wrote:
>> On 3/11/25 06:05, Nikunj A. Dadhania wrote:
>>> On 3/11/2025 2:41 PM, Nikunj A. Dadhania wrote:
>>>> On 3/10/2025 9:09 PM, Tom Lendacky wrote:
>>>>> On 3/10/25 01:45, Nikunj A Dadhania wrote:
>>>>
>>>>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>>>>>> index 50263b473f95..b61d6bd75b37 100644
>>>>>> --- a/arch/x86/kvm/svm/sev.c
>>>>>> +++ b/arch/x86/kvm/svm/sev.c
>>>>>> @@ -2205,6 +2205,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;
>
> The code just below this clobbers kvm->arch.default_tsc_khz, which could already
> have been set by userspace. Why? Either require params.desired_tsc_khz to match
> kvm->arch.default_tsc_khz, or have KVM's ABI be that KVM stuffs desired_tsc_khz
> based on kvm->arch.default_tsc_khz. I don't see any reason to add yet another
> way to control TSC.
Setting of the desired TSC frequency needs to be done during SNP_LAUNCH_START,
while parsing of the tsc-frequency happens as part of the cpu common class,
and kvm->arch.default_tsc_khz is set pretty late.
Intially, I had planned using "-cpu model,tsc-frequency=<>", but couldn't
find a way to get this parsed early during SNP_LAUNCH_START.
Regards,
Nikunj
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests
2025-03-11 15:33 ` Nikunj A. Dadhania
@ 2025-03-11 15:53 ` Sean Christopherson
2025-03-11 16:02 ` Nikunj A. Dadhania
0 siblings, 1 reply; 20+ messages in thread
From: Sean Christopherson @ 2025-03-11 15:53 UTC (permalink / raw)
To: Nikunj A. Dadhania
Cc: Tom Lendacky, pbonzini, kvm, santosh.shukla, bp, isaku.yamahata
On Tue, Mar 11, 2025, Nikunj A. Dadhania wrote:
>
>
> On 3/11/2025 8:22 PM, Sean Christopherson wrote:
> > On Tue, Mar 11, 2025, Tom Lendacky wrote:
> >> On 3/11/25 06:05, Nikunj A. Dadhania wrote:
> >>> On 3/11/2025 2:41 PM, Nikunj A. Dadhania wrote:
> >>>> On 3/10/2025 9:09 PM, Tom Lendacky wrote:
> >>>>> On 3/10/25 01:45, Nikunj A Dadhania wrote:
> >>>>
> >>>>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> >>>>>> index 50263b473f95..b61d6bd75b37 100644
> >>>>>> --- a/arch/x86/kvm/svm/sev.c
> >>>>>> +++ b/arch/x86/kvm/svm/sev.c
> >>>>>> @@ -2205,6 +2205,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;
> >
> > The code just below this clobbers kvm->arch.default_tsc_khz, which could already
> > have been set by userspace. Why? Either require params.desired_tsc_khz to match
> > kvm->arch.default_tsc_khz, or have KVM's ABI be that KVM stuffs desired_tsc_khz
> > based on kvm->arch.default_tsc_khz. I don't see any reason to add yet another
> > way to control TSC.
>
> Setting of the desired TSC frequency needs to be done during SNP_LAUNCH_START,
> while parsing of the tsc-frequency happens as part of the cpu common class,
> and kvm->arch.default_tsc_khz is set pretty late.
That's a QEMU problem, not a KVM problem, no?
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests
2025-03-11 15:53 ` Sean Christopherson
@ 2025-03-11 16:02 ` Nikunj A. Dadhania
0 siblings, 0 replies; 20+ messages in thread
From: Nikunj A. Dadhania @ 2025-03-11 16:02 UTC (permalink / raw)
To: Sean Christopherson
Cc: Tom Lendacky, pbonzini, kvm, santosh.shukla, bp, isaku.yamahata
On 3/11/2025 9:23 PM, Sean Christopherson wrote:
> On Tue, Mar 11, 2025, Nikunj A. Dadhania wrote:
>>
>>
>> On 3/11/2025 8:22 PM, Sean Christopherson wrote:
>>> On Tue, Mar 11, 2025, Tom Lendacky wrote:
>>>> On 3/11/25 06:05, Nikunj A. Dadhania wrote:
>>>>> On 3/11/2025 2:41 PM, Nikunj A. Dadhania wrote:
>>>>>> On 3/10/2025 9:09 PM, Tom Lendacky wrote:
>>>>>>> On 3/10/25 01:45, Nikunj A Dadhania wrote:
>>>>>>
>>>>>>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>>>>>>>> index 50263b473f95..b61d6bd75b37 100644
>>>>>>>> --- a/arch/x86/kvm/svm/sev.c
>>>>>>>> +++ b/arch/x86/kvm/svm/sev.c
>>>>>>>> @@ -2205,6 +2205,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;
>>>
>>> The code just below this clobbers kvm->arch.default_tsc_khz, which could already
>>> have been set by userspace. Why? Either require params.desired_tsc_khz to match
>>> kvm->arch.default_tsc_khz, or have KVM's ABI be that KVM stuffs desired_tsc_khz
>>> based on kvm->arch.default_tsc_khz. I don't see any reason to add yet another
>>> way to control TSC.
>>
>> Setting of the desired TSC frequency needs to be done during SNP_LAUNCH_START,
>> while parsing of the tsc-frequency happens as part of the cpu common class,
>> and kvm->arch.default_tsc_khz is set pretty late.
>
> That's a QEMU problem, not a KVM problem, no?
Yes.
I had missed that KVM_CAP_SET_TSC_KHZ can be a VM ioctl as well when
KVM_CAP_VM_TSC_CONTROL is set. Let me use this interface to set the TSC frequency
and then during SNP_LAUNCH_START use kvm->arch.default_tsc_khz when using SecureTSC.
Does that sound ok?
Regards,
Nikunj
^ permalink raw reply [flat|nested] 20+ messages in thread