[MODERATED] Re: [PATCH v4 8/8] [PATCH v4 8/8] Linux Patch #8

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Jun 27, 2018 at 03:05:02PM +0200, speck for Thomas Gleixner wrote:
> On Sat, 23 Jun 2018, speck for konrad.wilk_at_oracle.com wrote:
> > x86/KVM/VMX: Use MSR save list for IA32_FLUSH_CMD if required.
> > +
> > +	/*
> > +	 * If we enforce flushing the L1D cache on every VMENTER lets use the
> > +	 * MSR save list.
> > +	 */
> > +	if (vmx_l1d_cache_flush_req(&vmx->vcpu))
> > +		if (vmentry_l1d_flush == 2)
> > +			add_atomic_switch_msr(vmx, MSR_IA32_FLUSH_CMD,
> > +					      L1D_FLUSH, 0, true);
> 
> That's broken when the CPU does not have the FLUSH MSR, unless I'm missing
> something.

<nods>
It should have the check for the static_cpu_has(X86_FEATURE_FLUSH_L1D).

> 
> >  static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> > @@ -10079,7 +10095,7 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
> >  	evmcs_rsp = static_branch_unlikely(&enable_evmcs) ?
> >  		(unsigned long)&current_evmcs->host_rsp : 0;
> >  
> > -	if (vcpu->arch.flush_cache_req)
> > +	if (vcpu->arch.flush_cache_req && vmentry_l1d_flush != 1)
> >  		kvm_l1d_flush();
> 
> Huch? So above you use the MSR list for vmentry_l1d_flush == 2 and here you
> do the direct flush for vmentry_l1d_flush != 1. That should be == 1, right?

Yes in fact I vividely remember fixing this up as my debug patch has this
this extra logic (so that I could turn on/off the MSR list):

    if (vcpu->arch.flush_cache_req) {
+               if ((vmentry_l1d_flush == 2) && (!use_msr_save))
+                       kvm_l1d_flush();
+               else if (vmentry_l1d_flush == 1)
+                       kvm_l1d_flush();

and yet I managed to post the stale version. Arggh.


> 
> Thanks,
> 
> 	tglx