linux-kernel - Re: [PATCH v13 09/13] tsc: Use the GUEST_TSC

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: <25c87e59-970b-8058-6959-ddcf8ce09bbc@amd.com>
Date: Tue, 22 Oct 2024 09:54:28 +0530
From: "Nikunj A. Dadhania" <nikunj@....com>
To: Tom Lendacky <thomas.lendacky@....com>, linux-kernel@...r.kernel.org,
 bp@...en8.de, x86@...nel.org, kvm@...r.kernel.org
Cc: mingo@...hat.com, tglx@...utronix.de, dave.hansen@...ux.intel.com,
 pgonda@...gle.com, seanjc@...gle.com, pbonzini@...hat.com
Subject: Re: [PATCH v13 09/13] tsc: Use the GUEST_TSC_FREQ MSR for discovering
 TSC frequency

On 10/21/2024 8:03 PM, Tom Lendacky wrote:
> On 10/21/24 00:51, Nikunj A Dadhania wrote:
>> Calibrating the TSC frequency using the kvmclock is not correct for
>> SecureTSC enabled guests. Use the platform provided TSC frequency via the
>> GUEST_TSC_FREQ MSR (C001_0134h).
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@....com>
>> ---
>>  arch/x86/include/asm/sev.h |  2 ++
>>  arch/x86/coco/sev/core.c   | 16 ++++++++++++++++
>>  arch/x86/kernel/tsc.c      |  5 +++++
>>  3 files changed, 23 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
>> index 9169b18eeb78..34f7b9fc363b 100644
>> --- a/arch/x86/include/asm/sev.h
>> +++ b/arch/x86/include/asm/sev.h
>> @@ -536,6 +536,7 @@ static inline int handle_guest_request(struct snp_msg_desc *mdesc, u64 exit_code
>>  }
>>  
>>  void __init snp_secure_tsc_prepare(void);
>> +void __init securetsc_init(void);
>>  
>>  #else	/* !CONFIG_AMD_MEM_ENCRYPT */
>>  
>> @@ -584,6 +585,7 @@ static inline int handle_guest_request(struct snp_msg_desc *mdesc, u64 exit_code
>>  				       u32 resp_sz) { return -ENODEV; }
>>  
>>  static inline void __init snp_secure_tsc_prepare(void) { }
>> +static inline void __init securetsc_init(void) { }
> 
> This should probably be named snp_securetsc_init() or
> snp_secure_tsc_init() (to be consistent with the function above it) so
> that it is in the snp namespace.

Ack.

>> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
>> index dfe6847fd99e..c83f1091bb4f 100644
>> --- a/arch/x86/kernel/tsc.c
>> +++ b/arch/x86/kernel/tsc.c
>> @@ -30,6 +30,7 @@
>>  #include <asm/i8259.h>
>>  #include <asm/topology.h>
>>  #include <asm/uv/uv.h>
>> +#include <asm/sev.h>
>>  
>>  unsigned int __read_mostly cpu_khz;	/* TSC clocks / usec, not used here */
>>  EXPORT_SYMBOL(cpu_khz);
>> @@ -1514,6 +1515,10 @@ void __init tsc_early_init(void)
>>  	/* Don't change UV TSC multi-chassis synchronization */
>>  	if (is_early_uv_system())
>>  		return;
>> +
>> +	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
>> +		securetsc_init();
> 
> Would this call be better in kvm_init_platform() or kvmclock_init()? Any
> reason it has to be here?

Added here to make sure that initialization happens on all the hypervisor,
not just for KVM. Moreover, kvmclock can be disabled from the command line.

Regards,
Nikunj