Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Date:   Fri, 15 Sep 2023 07:51:59 -0700
In-Reply-To: <ZQRtpVjXTwjeJ5rI@google.com>
Mime-Version: 1.0
References: <cover.1694721045.git.thomas.lendacky@amd.com> <025fd734d35acbbbbca74c4b3ed671a02d4af628.1694721045.git.thomas.lendacky@amd.com>
 <ZQRtpVjXTwjeJ5rI@google.com>
Message-ID: <ZQRvjy/NQa3HcKsY@google.com>
Subject: Re: [PATCH 2/2] KVM: SVM: Do not use user return MSR support for
 virtualized TSC_AUX
From:   Sean Christopherson <seanjc@google.com>
To:     Tom Lendacky <thomas.lendacky@amd.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org, x86@kernel.org,
        Paolo Bonzini <pbonzini@redhat.com>,
        Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Ingo Molnar <mingo@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Babu Moger <babu.moger@amd.com>
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

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@amd.com>
> > ---
> >  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.