Re: [PATCH 2/2] KVM: SVM: Do not use user return MSR support for virtualized TSC_AUX

[Tom Lendacky]

On 9/15/23 09:51, Sean Christopherson wrote:
> On Fri, Sep 15, 2023, Sean Christopherson wrote:
> > On Thu, Sep 14, 2023, Tom Lendacky wrote:
> > > When the TSC_AUX MSR is virtualized, the TSC_AUX value is swap type "B"
> > > within the VMSA. This means that the guest value is loaded on VMRUN and
> > > the host value is restored from the host save area on #VMEXIT.
> > >
> > > Since the value is restored on #VMEXIT, the KVM user return MSR support
> > > for TSC_AUX can be replaced by populating the host save area with current
> > > host value of TSC_AUX. This replaces two WRMSR instructions with a single
> > > RDMSR instruction.
> > >
> > > Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> > > ---
> > >   arch/x86/kvm/svm/sev.c | 14 +++++++++++++-
> > >   arch/x86/kvm/svm/svm.c | 26 ++++++++++++++++----------
> > >   arch/x86/kvm/svm/svm.h |  4 +++-
> > >   3 files changed, 32 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > index 565c9de87c6d..1bbaae2fed96 100644
> > > --- a/arch/x86/kvm/svm/sev.c
> > > +++ b/arch/x86/kvm/svm/sev.c
> > > @@ -2969,6 +2969,7 @@ static void sev_es_init_vmcb_after_set_cpuid(struct vcpu_svm *svm)
> > >   	if (boot_cpu_has(X86_FEATURE_V_TSC_AUX) &&
> > >   	    (guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP) ||
> > >   	     guest_cpuid_has(vcpu, X86_FEATURE_RDPID))) {
> > > +		svm->v_tsc_aux = true;
> > >   		set_msr_interception(vcpu, svm->msrpm, MSR_TSC_AUX, 1, 1);
> > >   		if (guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
> > >   			svm_clr_intercept(svm, INTERCEPT_RDTSCP);
> > > @@ -3071,8 +3072,10 @@ void sev_es_vcpu_reset(struct vcpu_svm *svm)
> > >   					    sev_enc_bit));
> > >   }
> > >   
> > > -void sev_es_prepare_switch_to_guest(struct sev_es_save_area *hostsa)
> > > +void sev_es_prepare_switch_to_guest(struct vcpu_svm *svm, struct sev_es_save_area *hostsa)
> > >   {
> > > +	u32 msr_hi;
> > > +
> > >   	/*
> > >   	 * All host state for SEV-ES guests is categorized into three swap types
> > >   	 * based on how it is handled by hardware during a world switch:
> > > @@ -3109,6 +3112,15 @@ void sev_es_prepare_switch_to_guest(struct sev_es_save_area *hostsa)
> > >   		hostsa->dr2_addr_mask = amd_get_dr_addr_mask(2);
> > >   		hostsa->dr3_addr_mask = amd_get_dr_addr_mask(3);
> > >   	}
> > > +
> > > +	/*
> > > +	 * If TSC_AUX virtualization is enabled, MSR_TSC_AUX is loaded but NOT
> > > +	 * saved by the CPU (Type-B). If TSC_AUX is not virtualized, the user
> > > +	 * return MSR support takes care of restoring MSR_TSC_AUX. This
> > > +	 * exchanges two WRMSRs for one RDMSR.
> > > +	 */
> > > +	if (svm->v_tsc_aux)
> > > +		rdmsr(MSR_TSC_AUX, hostsa->tsc_aux, msr_hi);
> >
> > IIUC, when V_TSC_AUX is supported, SEV-ES guests context switch MSR_TSC_AUX
> > regardless of what has been exposed to the guest.  So rather than condition the
> > hostsa->tsc_aux update on guest CPUID, just do it if V_TSC_AUX is supported.
> >
> > And then to avoid the RDMSR, which is presumably the motivation for checking
> > guest CPUID, grab the host value from user return framework.  The host values
> > are per-CPU, but constant after boot, so the only requirement is that KVM sets
> > up MSR_TSC_AUX in the user return framework.
>
> Actually, duh.  The save area is also per-CPU, so just fill hostsa->tsc_aux in
> svm_hardware_setup() and then sev_es_prepare_switch_to_guest() never has to do
> anything.

Ah, right, because Linux never changes TSC_AUX post boot. Much simpler.

I'll rework based on the comments and send a v2 series.

Thanks,
Tom