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: <20241205110455.GCZ1GI1_vv5EIMJwXl@fat_crate.local>
Date: Thu, 5 Dec 2024 12:55:13 +0100
From: Borislav Petkov <bp@...en8.de>
To: Nikunj A Dadhania <nikunj@....com>
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 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. When did Peter test it again and
Tom review it again?

> 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)

> +		snp_tsc_scale = tsc_resp->tsc_scale;
> +		snp_tsc_offset = tsc_resp->tsc_offset;
> +	} else {
> +		pr_err("Failed to get TSC info, response status 0x%x\n", tsc_resp->status);
> +		rc = -EIO;
> +	}
> +
> +e_request:
> +	/* The response buffer contains sensitive data, explicitly clear it. */
> +	memzero_explicit(buf, sizeof(buf));
> +	memzero_explicit(tsc_resp, sizeof(*tsc_resp));
> +e_free_mdesc:
> +	snp_msg_free(mdesc);
> +e_free_buf:
> +	kfree(buf);
> +e_free_rio:
> +	kfree(rio);
> +e_free_req:
> +	kfree(req);
> + e_free_tsc_resp:
> +	kfree(tsc_resp);
> +e_free_tsc_req:
> +	kfree(tsc_req);
> +
> +	return rc;
> +}

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette