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
