On 8/20/2025 12:01 AM, Sean Christopherson wrote:
> On Mon, Aug 04, 2025, Nikunj A Dadhania wrote:
>> 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@xxxxxxx>
>> Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@xxxxxxx>
>> Reviewed-by: Kai Huang <kai.huang@xxxxxxxxx>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
>> Signed-off-by: Nikunj A Dadhania <nikunj@xxxxxxx>
>> ---
>> arch/x86/include/asm/svm.h | 1 +
>> arch/x86/kvm/svm/sev.c | 27 +++++++++++++++++++++++++++
>> arch/x86/kvm/svm/svm.c | 2 ++
>> arch/x86/kvm/svm/svm.h | 2 ++
>> 4 files changed, 32 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 e88dce598785..f9ab9ecc213f 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;
>> }
>>
>> +bool snp_secure_tsc_enabled(struct kvm *kvm)
>
> snp_is_secure_tsc_enabled() to make it super obvious this is a predicate.
Ack.
>
>> +{
>> + 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));
>
> Align indentation.
>
>> +}
>> @@ -4455,6 +4479,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));
>>
>> + if (snp_secure_tsc_enabled(vcpu->kvm))
>> + svm_disable_intercept_for_msr(vcpu, MSR_AMD64_GUEST_TSC_FREQ, MSR_TYPE_R);
>
> I'm leaning towards:
>
> svm_set_intercept_for_msr(vcpu, MSR_AMD64_GUEST_TSC_FREQ, MSR_TYPE_R,
> !snp_is_secure_tsc_enabled(vcpu->kvm));
>
> because the cost of setting a bit is negligible.
>
Ack.
>> +
>> /*
>> * For SEV-ES, accesses to MSR_IA32_XSS should not be intercepted if
>> * the host/guest supports its use.
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index d9931c6c4bc6..a81bf83ccb52 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -1317,6 +1317,8 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)
>>
>> svm->guest_state_loaded = false;
>>
>> + vcpu->arch.guest_tsc_protected = snp_secure_tsc_enabled(vcpu->kvm);
>
> Hmm, we can and should handle this in sev.c. If we add sev_vcpu_create(), then
> we don't need to expose snp_is_secure_tsc_enabled(), and we can move more code
> into that helper.
>
> I'll post a combined series of this and the GHCB version patches.
Thanks, I will test and get back on v11.
Regards
Nikunj
