Re: [PATCH v4 5/5] KVM: SVM: Enable Secure TSC for SNP guests

[Sean Christopherson <seanjc@google.com>]

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?