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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f2292bcb-ccc5-4121-98ce-bf65c0590131@amd.com>
Date: Thu, 26 Jun 2025 15:31:42 +0530
From: "Nikunj A. Dadhania" <nikunj@....com>
To: 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,
 thomas.lendacky@....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/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?

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