Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 1 Feb 2018 09:19:57 -0500
From:   Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To:     KarimAllah Ahmed <karahmed@amazon.com>
Cc:     Jim Mattson <jmattson@google.com>,
        KarimAllah Ahmed <karahmed@amazon.de>,
        kvm list <kvm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        the arch/x86 maintainers <x86@kernel.org>,
        Asit Mallick <asit.k.mallick@intel.com>,
        Arjan Van De Ven <arjan.van.de.ven@intel.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Dan Williams <dan.j.williams@intel.com>,
        Jun Nakajima <jun.nakajima@intel.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        David Woodhouse <dwmw@amazon.co.uk>,
        Greg KH <gregkh@linuxfoundation.org>,
        Andy Lutomirski <luto@kernel.org>,
        Ashok Raj <ashok.raj@intel.com>
Subject: Re: [PATCH v5 4/5] KVM: VMX: Allow direct access to
 MSR_IA32_SPEC_CTRL
Message-ID: <20180201141957.GC19621@char.us.oracle.com>
References: <1517427467-28567-1-git-send-email-karahmed@amazon.de>
 <1517427467-28567-5-git-send-email-karahmed@amazon.de>
 <CALMp9eRgc-qKS5CDTvqoUDdnAXu8xqy9ZB-rew=P9NgE5AfXWA@mail.gmail.com>
 <06cb88da-f355-41ed-380f-7daa8ddf6159@amazon.com>
 <CALMp9eS9LAJz+u3ujKZ5kLHihHMZ68LHajn3ri7sRPPEgFbHfA@mail.gmail.com>
 <a8f25ab3-0b1d-a248-9564-238e3dca3da4@amazon.com>
 <e28c13c4-4a04-8a41-2a5c-16509e1bad0e@amazon.com>
 <20180201042650.GD21751@char.us.oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180201042650.GD21751@char.us.oracle.com>
User-Agent: Mutt/1.8.3 (2017-05-23)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

.snip..
> > +/* Is SPEC_CTRL intercepted for the currently running vCPU? */
> > +static bool spec_ctrl_intercepted(struct kvm_vcpu *vcpu)
> > +{
> > +	unsigned long *msr_bitmap;
> > +	int f = sizeof(unsigned long);
> > +
> > +	if (!cpu_has_vmx_msr_bitmap())
> > +		return true;
> > +
> > +	msr_bitmap = is_guest_mode(vcpu) ?
> > +			to_vmx(vcpu)->nested.vmcs02.msr_bitmap :
> > +			to_vmx(vcpu)->vmcs01.msr_bitmap;
> > +
> > +	return !!test_bit(MSR_IA32_SPEC_CTRL, msr_bitmap + 0x800 / f);
> > +}
> > +
..snip..
> > @@ -3359,6 +3393,34 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  	case MSR_IA32_TSC:
> >  		kvm_write_tsc(vcpu, msr_info);
> >  		break;
> > +	case MSR_IA32_SPEC_CTRL:
> > +		if (!msr_info->host_initiated &&
> > +		    !guest_cpuid_has(vcpu, X86_FEATURE_IBRS) &&
> > +		    !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
> > +			return 1;
> > +
> > +		vmx->spec_ctrl_used = true;
> > +
> > +		/* The STIBP bit doesn't fault even if it's not advertised */
> > +		if (data & ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP))
> > +			return 1;
> > +
> > +		vmx->spec_ctrl = data;
> > +
> > +		/*
> > +		 * When it's written (to non-zero) for the first time, pass
> > +		 * it through. This means we don't have to take the perf
> 
> .. But only if it is a nested guest (as you have && is_guest_mode).
> 
> Do you want to update the comment a bit?
> 
> > +		 * hit of saving it on vmexit for the common case of guests
> > +		 * that don't use it.
> > +		 */
> > +		if (cpu_has_vmx_msr_bitmap() && data &&
> > +		    spec_ctrl_intercepted(vcpu) &&
> > +		    is_guest_mode(vcpu))
>                     ^^^^^^^^^^^^^^^^^^ <=== here

Would it be perhaps also good to mention the complexity of how
we ought to be handling L1 and L2 guests in the commit?

We are all stressed and I am sure some of us haven't gotten much
sleep - but it can help in say three months when some unluckly new
soul is trying to understand this and gets utterly confused.

> > +			vmx_disable_intercept_for_msr(
> > +					vmx->vmcs01.msr_bitmap,
> > +					MSR_IA32_SPEC_CTRL,
> > +					MSR_TYPE_RW);
> > +		break;
> >  	case MSR_IA32_PRED_CMD:
> >  		if (!msr_info->host_initiated &&
> >  		    !guest_cpuid_has(vcpu, X86_FEATURE_IBPB) &&