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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
>>>
>>> Do we need to perform any sanity checking against this value?
>>
>> On the higher side, sev-snp-guest.stsc-freq is u32, a Secure TSC guest boots fine with
>> TSC frequency set to the highest value (stsc-freq=0xFFFFFFFF).
>>
>> On the lower side as MSR_AMD64_GUEST_TSC_FREQ is in MHz, TSC clock should at least be 1Mhz. 
> 
> Something like this ?
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b61d6bd75b37..c46b6afa969d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2213,6 +2213,14 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  		if (!user_tsc_khz)
>  			user_tsc_khz = tsc_khz;
>  
> +		/*
> +		 * The minimum granularity for reporting Secure TSC frequency is
> +		 * 1MHz. Return an error if the user specifies a TSC frequency
> +		 * less than 1MHz.
> +		 */
> +		if (user_tsc_khz < 1000)
> +			return -EINVAL;

Seems reasonable to me. I'll let Sean or Paolo weigh in on it. I don't
think we need a message, there should be a check in the VMM, too, which
would be able to provide information to the end user?

Thanks,
Tom

> +
>  		start.desired_tsc_khz = user_tsc_khz;
>  
>  		/* Set the arch default TSC for the VM*/
> 
> Regards
> Nikunj




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux