Re: [PATCH] x86/sev: Use TSC_FACTOR for Secure TSC frequency calculation

[Nikunj A Dadhania <nikunj@amd.com>]

Nikunj A Dadhania <nikunj@amd.com> writes:

> When using Secure TSC, the GUEST_TSC_FREQ MSR reports a frequency based on
> the nominal P0 frequency, which deviates slightly (typically ~0.2%) from
> the actual mean TSC frequency due to clocking parameters. Over extended VM
> uptime, this discrepancy accumulates, causing clock skew between the
> hypervisor and SEV-SNP VM, leading to early timer interrupts as perceived
> by the guest.
>
> The guest kernel relies on the reported nominal frequency for TSC-based
> timekeeping, while the actual frequency set during SNP_LAUNCH_START may
> differ. This mismatch results in inaccurate time calculations, causing the
> guest to perceive hrtimers as firing earlier than expected.
>
> Utilize the TSC_FACTOR from the SEV firmware's secrets page (see "Secrets
> Page Format" in the SNP Firmware ABI Specification) to calculate the mean
> TSC frequency, ensuring accurate timekeeping and mitigating clock skew in
> SEV-SNP VMs.
>
> Use early_ioremap_encrypted() to map the secrets page as
> ioremap_encrypted() uses kmalloc() which is not available during early TSC
> initialization and causes a panic.
>
> Fixes: 73bbf3b0fbba ("x86/tsc: Init the TSC for Secure TSC guests")
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>

A gentle reminder for review.

Thanks
Nikunj

> ---
>  arch/x86/include/asm/sev.h |  5 ++++-
>  arch/x86/coco/sev/core.c   | 24 ++++++++++++++++++++++++
>  2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 58e028d42e41..655d7e37bbcc 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -282,8 +282,11 @@ struct snp_secrets_page {
>  	u8 svsm_guest_vmpl;
>  	u8 rsvd3[3];
>  
> +	/* The percentage decrease from nominal to mean TSC frequency. */
> +	u32 tsc_factor;
> +
>  	/* Remainder of page */
> -	u8 rsvd4[3744];
> +	u8 rsvd4[3740];
>  } __packed;
>  
>  struct snp_msg_desc {
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index b6db4e0b936b..ffd44712cec0 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -2167,15 +2167,39 @@ static unsigned long securetsc_get_tsc_khz(void)
>  
>  void __init snp_secure_tsc_init(void)
>  {
> +	struct snp_secrets_page *secrets;
>  	unsigned long long tsc_freq_mhz;
> +	void *mem;
>  
>  	if (!cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
>  		return;
>  
> +	mem = early_memremap_encrypted(sev_secrets_pa, PAGE_SIZE);
> +	if (!mem) {
> +		pr_err("Unable to get TSC_FACTOR: failed to map the SNP secrets page.\n");
> +		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECURE_TSC);
> +	}
> +
> +	secrets = (__force struct snp_secrets_page *)mem;
> +
>  	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
>  	rdmsrq(MSR_AMD64_GUEST_TSC_FREQ, tsc_freq_mhz);
>  	snp_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000);
>  
> +	/*
> +	 * 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;
> +	 */
> +	snp_tsc_freq_khz -= (snp_tsc_freq_khz * secrets->tsc_factor) / 100000;
> +
>  	x86_platform.calibrate_cpu = securetsc_get_tsc_khz;
>  	x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
> +
> +	early_memunmap(mem, PAGE_SIZE);
>  }
>
> base-commit: 337964c8abfbef645cbbe25245e25c11d9d1fc4c
> -- 
> 2.43.0