LinuxLists.cc - [PATCH v3 0/4] Speculation control improvements

2018-02-20 08:55:03

On Tue, 2018-02-20 at 11:42 +0100, Thomas Gleixner wrote:
>
> > > However, Paolo is very insistent that taking the trap every time is
> > > actually a lot *slower* than really frobbing IBRS on certain
> > > microarchitectures, so my hand-waving "pfft, what did they expect?" is
> > > not acceptable.
> > >
> > > Which I think puts us back to the "throwing the toys out of the pram"
>
> There are no more toys in the pram. I threw them all out weeks ago ...

One option is to take the patch as-is¹ with the trap on every access.
As soon as Intel define that 'IBRS_ALL_AND_THE_BIT_IS_A_NOOP' bit in
MSR_IA32_ARCH_CAPABILITIES, *then* we can expose it to guests directly
again just as we do at the moment.

That way, the slowdown that Paolo is concerned about is limited to a
small set of current CPUs on which we're mostly unlikely to care too
much about KVM anyway.

¹ as-is, except I really will go and turn the IA32_ARCH_CAPABILITIES
bits into scattered cpufeatures before I repost it.

Attachments:

smime.p7s (5.09 kB)

2018-02-20 11:27:34

by Paolo Bonzini

[permalink] [raw]

Subject: Re: [PATCH v3 2/4] x86/speculation: Support "Enhanced IBRS" on future CPUs

On 19/02/2018 11:50, David Woodhouse wrote:
> Cope with this by trapping and emulating *all* access to SPEC_CTRL from
> KVM guests when the IBRS_ALL feature is present, so it can never be
> turned off. Guests who see IBRS_ALL should never do anything except
> turn it on at boot anyway. And if they didn't know about IBRS_ALL and
> they keep frobbing IBRS on every kernel entry/exit... well the vmexit
> for a no-op is probably going to be faster than they were expecting
> anyway, so they'll live.

The problem is, it isn't. On a Haswell (which has fairly slow
SPEC_CTRL) toggling IBRS is 200 cycles. This gives a context switch
time of around 2000 clock cycles with PTI enabled.

This is fairly awful, but with a vmexit cost of ~1100 cycles that goes
up to 2000+(1100-200)*2 = 3800. That's more or less doubling the cost
of a system call.

With newer machines SPEC_CTRL cost goes down but vmexit cost doesn't, so
it's only worse.

For now, we really should do something like

if (vmx->spec_ctrl != host_spec_ctrl)
wrmsrl(MSR_IA32_SPEC_CTRL, host_spec_ctrl);
else
lfence();

which later can become

if (vmx->spec_ctrl != host_spec_ctrl)
wrmsrl(MSR_IA32_SPEC_CTRL, host_spec_ctrl);
else {
/* lfence not needed if host_spec_ctrl == 0 */
if (static_cpu_has(BUG_REALLY_WANTS_IBRS))
nospec_barrier();
}

Paolo

2018-02-20 11:30:04

by Paolo Bonzini

[permalink] [raw]

Subject: Re: [PATCH v3 2/4] x86/speculation: Support "Enhanced IBRS" on future CPUs

On 20/02/2018 12:22, David Woodhouse wrote:
>>>> However, Paolo is very insistent that taking the trap every time is
>>>> actually a lot *slower* than really frobbing IBRS on certain
>>>> microarchitectures, so my hand-waving "pfft, what did they expect?" is
>>>> not acceptable.
>>>>
>>>> Which I think puts us back to the "throwing the toys out of the pram"
>> There are no more toys in the pram. I threw them all out weeks ago ...
>
> One option is to take the patch as-is¹ with the trap on every access.

Please reword the commit message at least, mentioning that the slow case
is not one we don't care much about yet (no IBRS_ALL CPUs in the wild
afaik) and we won't care much about in the long run either (IBRS_ALL
really only used on a handful of blacklisted processors).

Thanks,

Paolo

> As soon as Intel define that 'IBRS_ALL_AND_THE_BIT_IS_A_NOOP' bit in
> MSR_IA32_ARCH_CAPABILITIES, *then* we can expose it to guests directly
> again just as we do at the moment.
>
> That way, the slowdown that Paolo is concerned about is limited to a
> small set of current CPUs on which we're mostly unlikely to care too
> much about KVM anyway.

2018-02-26 19:56:05

by Thomas Gleixner

[permalink] [raw]

Subject: Re: [PATCH v3 2/4] x86/speculation: Support "Enhanced IBRS" on future CPUs

On Tue, 20 Feb 2018, David Woodhouse wrote:
> On Tue, 2018-02-20 at 11:42 +0100, Thomas Gleixner wrote:
> >
> > > > However, Paolo is very insistent that taking the trap every time is
> > > > actually a lot *slower* than really frobbing IBRS on certain
> > > > microarchitectures, so my hand-waving "pfft, what did they expect?" is
> > > > not acceptable.
> > > >
> > > > Which I think puts us back to the "throwing the toys out of the pram"
> >
> > There are no more toys in the pram. I threw them all out weeks ago ...
>
> One option is to take the patch as-is¹ with the trap on every access.
> As soon as Intel define that 'IBRS_ALL_AND_THE_BIT_IS_A_NOOP' bit in
> MSR_IA32_ARCH_CAPABILITIES, *then* we can expose it to guests directly
> again just as we do at the moment.

Arjan, is there any update on this?

Thanks,

tglx