linux-kernel - Re: [PATCH 2/2] KVM: SVM: Use cached value as restore value of 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: <9991df11-fe7c-41e1-9890-f0c38adc8137@amd.com>
Date: Thu, 18 Sep 2025 13:47:06 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Hou Wenlong <houwenlong.hwl@...group.com>, kvm@...r.kernel.org
Cc: Lai Jiangshan <jiangshan.ljs@...group.com>,
 Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini
 <pbonzini@...hat.com>, Thomas Gleixner <tglx@...utronix.de>,
 Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
 Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
 "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] KVM: SVM: Use cached value as restore value of
 TSC_AUX for SEV-ES guest

On 9/17/25 22:38, Hou Wenlong wrote:
> The commit 916e3e5f26ab ("KVM: SVM: Do not use user return MSR support
> for virtualized TSC_AUX") assumes that TSC_AUX is not changed by Linux
> post-boot, so it always restores the initial host value on #VMEXIT.
> However, this is not true in KVM, as it can be modified by user return
> MSR support for normal guests. If an SEV-ES guest always restores the
> initial host value on #VMEXIT, this may result in the cached value in
> user return MSR being different from the hardware value if the previous
> vCPU was a non-SEV-ES guest that had called kvm_set_user_return_msr().
> Consequently, this may pose a problem when switching back to that vCPU,
> as kvm_set_user_return_msr() would not update the hardware value because
> the cached value matches the target value. Unlike the TDX case, the
> SEV-ES guest has the ability to set the restore value in the host save
> area, and the cached value in the user return MSR is always the current
> hardware value. Therefore, the cached value could be used directly
> without RDMSR in svm_prepare_switch_to_guest(), making this change
> minimal.

I'm not sure I follow. If Linux never changes the value of TSC_AUX once it
has set it, then how can it ever be different? Have you seen this issue?

Thanks,
Tom

> 
> Fixes: 916e3e5f26ab ("KVM: SVM: Do not use user return MSR support for virtualized TSC_AUX")
> Suggested-by: Lai Jiangshan <jiangshan.ljs@...group.com>
> Signed-off-by: Hou Wenlong <houwenlong.hwl@...group.com>
> ---
>  arch/x86/kvm/svm/svm.c | 33 ++++++++++++++-------------------
>  1 file changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 1650de78648a..1be9c65ee23b 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -577,18 +577,6 @@ static int svm_enable_virtualization_cpu(void)
>  
>  	amd_pmu_enable_virt();
>  
> -	/*
> -	 * If TSC_AUX virtualization is supported, TSC_AUX becomes a swap type
> -	 * "B" field (see sev_es_prepare_switch_to_guest()) for SEV-ES guests.
> -	 * Since Linux does not change the value of TSC_AUX once set, prime the
> -	 * TSC_AUX field now to avoid a RDMSR on every vCPU run.
> -	 */
> -	if (boot_cpu_has(X86_FEATURE_V_TSC_AUX)) {
> -		u32 __maybe_unused msr_hi;
> -
> -		rdmsr(MSR_TSC_AUX, sev_es_host_save_area(sd)->tsc_aux, msr_hi);
> -	}
> -
>  	return 0;
>  }
>  
> @@ -1408,12 +1396,19 @@ static void svm_prepare_switch_to_guest(struct kvm_vcpu *vcpu)
>  	/*
>  	 * TSC_AUX is always virtualized for SEV-ES guests when the feature is
>  	 * available. The user return MSR support is not required in this case
> -	 * because TSC_AUX is restored on #VMEXIT from the host save area
> -	 * (which has been initialized in svm_enable_virtualization_cpu()).
> +	 * because TSC_AUX is restored on #VMEXIT from the host save area.
> +	 * However, user return MSR could change the value of TSC_AUX in the
> +	 * kernel. Therefore, to maintain the logic of user return MSR, set the
> +	 * restore value to the cached value of user return MSR, which should
> +	 * always reflect the current hardware value.
>  	 */
> -	if (likely(tsc_aux_uret_slot >= 0) &&
> -	    (!boot_cpu_has(X86_FEATURE_V_TSC_AUX) || !sev_es_guest(vcpu->kvm)))
> -		kvm_set_user_return_msr(tsc_aux_uret_slot, svm->tsc_aux, -1ull);
> +	if (likely(tsc_aux_uret_slot >= 0)) {
> +		if (!boot_cpu_has(X86_FEATURE_V_TSC_AUX) || !sev_es_guest(vcpu->kvm))
> +			kvm_set_user_return_msr(tsc_aux_uret_slot, svm->tsc_aux, -1ull);
> +		else
> +			sev_es_host_save_area(sd)->tsc_aux =
> +				(u32)kvm_get_user_return_msr_cache(tsc_aux_uret_slot);
> +	}
>  
>  	if (cpu_feature_enabled(X86_FEATURE_SRSO_BP_SPEC_REDUCE) &&
>  	    !sd->bp_spec_reduce_set) {
> @@ -3004,8 +2999,8 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  		 * TSC_AUX is always virtualized for SEV-ES guests when the
>  		 * feature is available. The user return MSR support is not
>  		 * required in this case because TSC_AUX is restored on #VMEXIT
> -		 * from the host save area (which has been initialized in
> -		 * svm_enable_virtualization_cpu()).
> +		 * from the host save area (which has been set in
> +		 * svm_prepare_switch_to_guest()).
>  		 */
>  		if (boot_cpu_has(X86_FEATURE_V_TSC_AUX) && sev_es_guest(vcpu->kvm))
>  			break;