Re: [PATCH v8 15/17] x86/ctxt: Issue a speculation barrier between vcpu contexts

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2516 bytes --]

On Mon, 2018-01-15 at 13:02 +0000, Andrew Cooper wrote:
> On 15/01/18 12:54, David Woodhouse wrote:
> > 
> > On Fri, 2018-01-12 at 18:01 +0000, Andrew Cooper wrote:
> > > 
> > > @@ -1736,6 +1736,9 @@ void context_switch(struct vcpu *prev, struct
> > > vcpu *next)
> > >          }
> > >  
> > >          ctxt_switch_levelling(next);
> > > +
> > > +        if ( opt_ibpb )
> > > +            wrmsrl(MSR_PRED_CMD, PRED_CMD_IBPB);
> > >      }
> > > 
> > If you're doing that without an 'else lfence' don't you need to include
> > a comment with your proof that it's safe to do so, and the CPU can't
> > speculate a taken conditional branch and all the way to a problematic
> > instruction?
>
> What would that gain?  A malicious guest could speculate around it, but
> speculation will catch up (at the very latest) when we return to guest,
> which is a serialising event.

There's your proof. I'd just be happier with a blanket policy of
*including* that proof in all cases where we do this kind of runtime
conditional branch around setting IBRS or IBPB. Because if we're in the
habit of doing the 'if (foo) wrmsrl()' without it, we *might* miss a
case where it's not actually OK.

(Aside: Is VMLAUNCH actually architecturally guaranteed to be
serialising? That doesn't seem consistent with a long-term goal of
designing hardware to make VMs go faster. Or have we merely extracted a
promise from Intel that *current* hardware will stop speculative
execution when it hits a VMLAUNCH?)
 
> > 
> > Also... if you're doing that in context_switch() does it do the right
> > thing with idle? If a CPU switches to the idle domain and then back
> > again to the same vCPU, does that do the IBPB twice?
>
> Context switches to idle will skip the IBPB because it isn't needed, but
> any switch to non-idle need it.  In your example, we should execute just
> a single IBPB.

In my example I think we should not execute IBPB at all. We come from a
given VMCS, sleep for a while, and go back to it. No need for any
flushing whatsoever.

> > 
> > For vmx we only really need IBPB when we do VMPTRLD, right?
>
> That is part of IBRS_ATT is it not?

It doesn't go away with IBRS_ALL (as it's now called), but it's the
same for the existing IBRS *and* retpoline. Doing it on VMPTRLD is what
Linux is doing.

(cf. https://lkml.org/lkml/2018/1/13/40 and
     https://lkml.org/lkml/2018/1/15/146 and note the AMD SVM caveat.)



[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel