[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250919131535.GA73646@k08j02272.eu95sqa>
Date: Fri, 19 Sep 2025 21:15:35 +0800
From: Hou Wenlong <houwenlong.hwl@...group.com>
To: Tom Lendacky <thomas.lendacky@....com>
Cc: kvm@...r.kernel.org, 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 Thu, Sep 18, 2025 at 01:47:06PM -0500, Tom Lendacky wrote:
> 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
>
Hi, Tom.
IIUD, the normal guest still uses the user return MSR to load the guest
TSC_AUX value into the hardware when TSC_AUX virtualization is
supported. However, the user return MSR only restores the host value
when returning to userspace, rather than when the vCPU is scheduled out.
This may lead to an issue during vCPU switching on a single pCPU, which
appears as follows:
normal vCPU -> SEV-ES vCPU -> normal vCPU
When the normal vCPU switches to the SEV-ES vCPU, the hardware TSC_AUX
value remains as the guest value set in kvm_set_user_return_msr() by the
normal vCPU. After the #VMEXIT from the SEV-ES vCPU, the hardware value
becomes the host value. However, the cached TSC_AUX value in the user
return MSR remains the guest value of previous normal vCPU. Therefore,
when switching back to that normal vCPU, kvm_set_user_return_msr() does
not perform a WRMSR to load the guest value into the hardware, because
the cached value matches the target value. As a result, during the
execution of the normal vCPU, the normal vCPU would get an incorrect
TSC_AUX value for RDTSCP/RDPID.
I didn't find the available description of TSC_AUX virtualization in
APM; all my analysis is based on the current KVM code. Am I missing
something?
Thanks.
> >
> > 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;
Powered by blists - more mailing lists