linux-kernel - Re: [PATCH v2] x86/sev: Use TSC_FACTOR for Secure TSC frequency calculation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <069049ff-178a-6d94-d161-5a7b90b6245c@amd.com>
Date: Thu, 26 Jun 2025 08:41:44 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: "Nikunj A. Dadhania" <nikunj@....com>, Ingo Molnar <mingo@...nel.org>
Cc: linux-kernel@...r.kernel.org, bp@...en8.de, x86@...nel.org,
 tglx@...utronix.de, mingo@...hat.com, dave.hansen@...ux.intel.com,
 aik@....com, dionnaglaze@...gle.com, stable@...r.kernel.org
Subject: Re: [PATCH v2] x86/sev: Use TSC_FACTOR for Secure TSC frequency
 calculation

On 6/26/25 05:01, Nikunj A. Dadhania wrote:
> 
> 
> On 6/26/2025 1:56 PM, Ingo Molnar wrote:
>>
>> * Nikunj A Dadhania <nikunj@....com> wrote:
>>
>>> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
>>> index fbb616fcbfb8..869355367210 100644
>>> --- a/arch/x86/include/asm/sev.h
>>> +++ b/arch/x86/include/asm/sev.h
>>> @@ -223,6 +223,19 @@ struct snp_tsc_info_resp {
>>>  	u8 rsvd2[100];
>>>  } __packed;
>>>  
>>> +
>>> +/*
>>> + * Obtain the mean TSC frequency by decreasing the nominal TSC frequency with
>>> + * TSC_FACTOR as documented in the SNP Firmware ABI specification:
>>> + *
>>> + * GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001))
>>> + *
>>> + * which is equivalent to:
>>> + *
>>> + * GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000;
>>> + */
>>> +#define SNP_SCALE_TSC_FREQ(freq, factor) ((freq) - ((freq) * (factor)) / 100000)
>>
>> Nit: there's really no need to use parentheses in this expression,
>> 'x * y / z' is equivalent and fine.
> 
> It will give wrong scale if I call with freq as "tsc + 1000000" 
> without the parentheses?

I think Ingo is saying this can be ((freq) - (freq) * (factor) / 100000)

in other words, getting rid of the parentheses around the multiplication.

Thanks,
Tom

> 
> SNP_SCALE_TSC_FREQ(tsc + 1000000, factor)
> 
>>> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
>>> index 8375ca7fbd8a..36f419ff25d4 100644
>>> --- a/arch/x86/coco/sev/core.c
>>> +++ b/arch/x86/coco/sev/core.c
>>> @@ -2156,20 +2156,32 @@ void __init snp_secure_tsc_prepare(void)
>>>  
>>>  static unsigned long securetsc_get_tsc_khz(void)
>>>  {
>>> -	return snp_tsc_freq_khz;
>>> +	return (unsigned long)snp_tsc_freq_khz;
>>
>> This forced type cast is a signature of poor type choices. Please 
>> harmonize the types of snp_tsc_freq_khz and securetsc_get_tsc_khz() to 
>> avoid the type cast altogether. 
> 
> Sure, I can attempt that and send an updated patch.
> 
>> Does this code even get built and run on 32-bit kernels?
> 
> This code should not build for 32-bit kernels.
> 
> Thanks
> Nikunj