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

[Nikunj A Dadhania <nikunj@amd.com>]

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