linux-kernel - Re: [PATCH v15 03/13] x86/sev: Add Secure TSC support for SNP guests

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: <7925f569-f660-420d-abdd-32a3bbcde164@amd.com>
Date: Fri, 6 Dec 2024 09:49:19 +0530
From: "Nikunj A. Dadhania" <nikunj@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: linux-kernel@...r.kernel.org, thomas.lendacky@....com, x86@...nel.org,
 kvm@...r.kernel.org, mingo@...hat.com, tglx@...utronix.de,
 dave.hansen@...ux.intel.com, pgonda@...gle.com, seanjc@...gle.com,
 pbonzini@...hat.com
Subject: Re: [PATCH v15 03/13] x86/sev: Add Secure TSC support for SNP guests



On 12/5/2024 5:25 PM, Borislav Petkov wrote:
> On Tue, Dec 03, 2024 at 02:30:35PM +0530, Nikunj A Dadhania wrote:
>> Add support for Secure TSC in SNP-enabled guests. Secure TSC allows guests
>> to securely use RDTSC/RDTSCP instructions, ensuring that the parameters
>> used cannot be altered by the hypervisor once the guest is launched.
>>
>> Secure TSC-enabled guests need to query TSC information from the AMD
>> Security Processor. This communication channel is encrypted between the AMD
>> Security Processor and the guest, with the hypervisor acting merely as a
>> conduit to deliver the guest messages to the AMD Security Processor. Each
>> message is protected with AEAD (AES-256 GCM).
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@....com>
>> Tested-by: Peter Gonda <pgonda@...gle.com>
> 
> 
>> Reviewed-by: Tom Lendacky <thomas.lendacky@....com>
> 
> This patch changed somewhat from last time. 

Yes, most of the change was dynamic allocation in snp_get_tsc_info().

> When did Peter test it again and
> Tom review it again?

It makes sense to drop both.

> 
>> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
>> index a61898c7f114..39683101b526 100644
>> --- a/arch/x86/coco/sev/core.c
>> +++ b/arch/x86/coco/sev/core.c
>> @@ -96,6 +96,14 @@ static u64 sev_hv_features __ro_after_init;
>>  /* Secrets page physical address from the CC blob */
>>  static u64 secrets_pa __ro_after_init;
>>  
>> +/*
>> + * For Secure TSC guests, the BP fetches TSC_INFO using SNP guest messaging and
> 
> s/BP/BSP/
> 
>> + * initializes snp_tsc_scale and snp_tsc_offset. These values are replicated
>> + * across the APs VMSA fields (TSC_SCALE and TSC_OFFSET).
>> + */
>> +static u64 snp_tsc_scale __ro_after_init;
>> +static u64 snp_tsc_offset __ro_after_init;
>> +
>>  /* #VC handler runtime per-CPU data */
>>  struct sev_es_runtime_data {
>>  	struct ghcb ghcb_page;
> 
> ...
> 
>> +	memcpy(tsc_resp, buf, sizeof(*tsc_resp));
>> +	pr_debug("%s: response status 0x%x scale 0x%llx offset 0x%llx factor 0x%x\n",
>> +		 __func__, tsc_resp->status, tsc_resp->tsc_scale, tsc_resp->tsc_offset,
>> +		 tsc_resp->tsc_factor);
>> +
>> +	if (tsc_resp->status == 0) {
> 
> Like the last time:
> 
> 	if (!tsc_resp->status)

Ack.
 
Regards
Nikunj