From: Nikunj A Dadhania <nikunj@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.
Always use kvm->arch.arch.default_tsc_khz as the TSC frequency that is
passed to SNP guests in the SNP_LAUNCH_START command. The default value
is the host TSC frequency. The userspace can optionally change the TSC
frequency via the KVM_SET_TSC_KHZ ioctl before calling the
SNP_LAUNCH_START ioctl.
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.
Co-developed-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
[sean: contain Secure TSC to sev.c]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index ffc27f676243..17f6c3fedeee 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -299,6 +299,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 VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 7d1d34e45310..fb45a96e0159 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -146,6 +146,14 @@ static bool sev_vcpu_has_debug_swap(struct vcpu_svm *svm)
return sev->vmsa_features & SVM_SEV_FEAT_DEBUG_SWAP;
}
+static bool snp_is_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));
+}
+
/* Must be called with the sev_bitmap_lock held */
static bool __sev_recycle_asids(unsigned int min_asid, unsigned int max_asid)
{
@@ -415,6 +423,9 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
if (data->flags)
return -EINVAL;
+ if (!snp_active)
+ valid_vmsa_features &= ~SVM_SEV_FEAT_SECURE_TSC;
+
if (data->vmsa_features & ~valid_vmsa_features)
return -EINVAL;
@@ -2195,6 +2206,12 @@ 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_is_secure_tsc_enabled(kvm)) {
+ WARN_ON_ONCE(!kvm->arch.default_tsc_khz);
+ 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) {
@@ -3085,6 +3102,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 && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
}
void sev_hardware_unsetup(void)
@@ -4452,6 +4472,9 @@ void sev_es_recalc_msr_intercepts(struct kvm_vcpu *vcpu)
!guest_cpu_cap_has(vcpu, X86_FEATURE_RDTSCP) &&
!guest_cpu_cap_has(vcpu, X86_FEATURE_RDPID));
+ svm_set_intercept_for_msr(vcpu, MSR_AMD64_GUEST_TSC_FREQ, MSR_TYPE_R,
+ !snp_is_secure_tsc_enabled(vcpu->kvm));
+
/*
* For SEV-ES, accesses to MSR_IA32_XSS should not be intercepted if
* the host/guest supports its use.
@@ -4591,6 +4614,9 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
return -ENOMEM;
svm->sev_es.vmsa = page_address(vmsa_page);
+
+ vcpu->arch.guest_tsc_protected = snp_is_secure_tsc_enabled(vcpu->kvm);
+
return 0;
}
--
2.51.0.rc1.167.g924127e9c0-goog
On 8/20/2025 5:18 AM, Sean Christopherson wrote:
> From: Nikunj A Dadhania <nikunj@amd.com>
>
> @@ -2195,6 +2206,12 @@ 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_is_secure_tsc_enabled(kvm)) {
> + WARN_ON_ONCE(!kvm->arch.default_tsc_khz);
Any particular reason to drop the the following change:
+ if (WARN_ON(!kvm->arch.default_tsc_khz)) {
+ rc = -EINVAL;
+ goto e_free_context;
+ }
As this is an unsupported configuration as per the SEV SNP Firmware ABI Specification:
8.16 SNP_LAUNCH_START
DESIRED_TSC_FREQ
Hypervisor-desired mean TSC frequency in KHz of the guest. This field has no effect if guests
do not enable Secure TSC in the VMSA. The hypervisor should set this field to 0h if it
*does not support Secure TSC* for this guest.
> + start.desired_tsc_khz = kvm->arch.default_tsc_khz;
> + }
> +
Regards,Nikunj
On Wed, Aug 20, 2025, Nikunj A. Dadhania wrote:
>
>
> On 8/20/2025 5:18 AM, Sean Christopherson wrote:
> > From: Nikunj A Dadhania <nikunj@amd.com>
> >
> > @@ -2195,6 +2206,12 @@ 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_is_secure_tsc_enabled(kvm)) {
> > + WARN_ON_ONCE(!kvm->arch.default_tsc_khz);
>
> Any particular reason to drop the the following change:
>
> + if (WARN_ON(!kvm->arch.default_tsc_khz)) {
> + rc = -EINVAL;
> + goto e_free_context;
> + }
Based on this conversation[*], both Kai and I expected KVM to let firmware deal
with the should-be-impossible situation.
On Tue, Jul 8, 2025 at 9:15 PM Nikunj A. Dadhania <nikunj@amd.com> wrote:
> On 7/8/2025 8:04 PM, Sean Christopherson wrote:
> > On Tue, Jul 08, 2025, Kai Huang wrote:
> >>>> Even some bug results in the default_tsc_khz being 0, will the
> >>>> SNP_LAUNCH_START command catch this and return error?
> >>>
> >>> No, that is an invalid configuration, desired_tsc_khz is set to 0 when
> >>> SecureTSC is disabled. If SecureTSC is enabled, desired_tsc_khz should
> >>> have correct value.
> >>
> >> So it's an invalid configuration that when Secure TSC is enabled and
> >> desired_tsc_khz is 0. Assuming the SNP_LAUNCH_START will return an error
> >> if such configuration is used, wouldn't it be simpler if you remove the
> >> above check and depend on the SNP_LAUNCH_START command to catch the
> >> invalid configuration?
> >
> > Support for secure TSC should depend on tsc_khz being non-zero. That way it'll
> > be impossible for arch.default_tsc_khz to be zero at runtime. Then KVM can WARN
> > on arch.default_tsc_khz being zero during SNP_LAUNCH_START.
>
> Sure.
https://lore.kernel.org/all/c327df02-c2eb-41e7-9402-5a16aa211265@amd.com
>
> As this is an unsupported configuration as per the SEV SNP Firmware ABI Specification:
Right, but what happens if KVM manages to pass in '0' for the frequency? Does
SNP_LAUNCH_START fail? If so, bailing from KVM doesn't seem to add any value.
>
> 8.16 SNP_LAUNCH_START
>
> DESIRED_TSC_FREQ
> Hypervisor-desired mean TSC frequency in KHz of the guest. This field has no
> effect if guests do not enable Secure TSC in the VMSA. The hypervisor should
> set this field to 0h if it *does not support Secure TSC* for this guest.
>
> > + start.desired_tsc_khz = kvm->arch.default_tsc_khz;
> > + }
> > +
> Regards,Nikunj
On 8/20/2025 6:31 PM, Sean Christopherson wrote:
> On Wed, Aug 20, 2025, Nikunj A. Dadhania wrote:
>>
>>
>> On 8/20/2025 5:18 AM, Sean Christopherson wrote:
>>> From: Nikunj A Dadhania <nikunj@amd.com>
>>>
>>> @@ -2195,6 +2206,12 @@ 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_is_secure_tsc_enabled(kvm)) {
>>> + WARN_ON_ONCE(!kvm->arch.default_tsc_khz);
>>
>> Any particular reason to drop the the following change:
>>
>> + if (WARN_ON(!kvm->arch.default_tsc_khz)) {
>> + rc = -EINVAL;
>> + goto e_free_context;
>> + }
>
> Based on this conversation[*], both Kai and I expected KVM to let firmware deal
> with the should-be-impossible situation.
>
> On Tue, Jul 8, 2025 at 9:15 PM Nikunj A. Dadhania <nikunj@amd.com> wrote:
> > On 7/8/2025 8:04 PM, Sean Christopherson wrote:
> > > On Tue, Jul 08, 2025, Kai Huang wrote:
> > >>>> Even some bug results in the default_tsc_khz being 0, will the
> > >>>> SNP_LAUNCH_START command catch this and return error?
> > >>>
> > >>> No, that is an invalid configuration, desired_tsc_khz is set to 0 when
> > >>> SecureTSC is disabled. If SecureTSC is enabled, desired_tsc_khz should
> > >>> have correct value.
> > >>
> > >> So it's an invalid configuration that when Secure TSC is enabled and
> > >> desired_tsc_khz is 0. Assuming the SNP_LAUNCH_START will return an error
> > >> if such configuration is used, wouldn't it be simpler if you remove the
> > >> above check and depend on the SNP_LAUNCH_START command to catch the
> > >> invalid configuration?
> > >
> > > Support for secure TSC should depend on tsc_khz being non-zero. That way it'll
> > > be impossible for arch.default_tsc_khz to be zero at runtime. Then KVM can WARN
> > > on arch.default_tsc_khz being zero during SNP_LAUNCH_START.
> >
> > Sure.
>
> https://lore.kernel.org/all/c327df02-c2eb-41e7-9402-5a16aa211265@amd.com
>
>>
>> As this is an unsupported configuration as per the SEV SNP Firmware ABI Specification:
>
> Right, but what happens if KVM manages to pass in '0' for the frequency? Does
> SNP_LAUNCH_START fail?
SNP_LAUNCH_START succeeds, and the guest kernel starts and panics during early boot [*]
> If so, bailing from KVM doesn't seem to add any value.
As firmware does not bail out, I had kept this check.
RegardsNikunjhttps://lore.kernel.org/all/afcf9a0b-7450-4df7-a21b-80b56264fc15@amd.com
© 2016 - 2025 Red Hat, Inc.