Re: [PATCH 4/7] x86/idle: Disable IBRS entering idle and enable it on wakeup

Peter Zijlstra
2018-01-08T08:24:25Z

On Thu, Jan 04, 2018 at 03:22:09PM -0800, Tim Chen wrote:
> >> +static inline void unprotected_speculation_end(void)
> >> +{
> >> + if (boot_cpu_has(X86_FEATURE_SPEC_CTRL))
> >> + __disable_indirect_speculation();
> >> + else
> >> + /*
> >> + * If we intended to disable indirect speculation
> >> + * but come here due to mis-speculation, we need
> >> + * to stop the mis-speculation with rmb.
> >> + */
> >> + rmb();
> >
> > Code is lacking {}, also the comment doesn't make sense. If we don't
> > have the MSR, why are we doing an LFENCE?
>
> The reason for lfence is because we intend to put up IBRS and disable
> speculation. But if the CPU mis-speculates into the else portion,
> we could be speculating without IBRS. The lfence stop the mis-speculation.

With static_cpu_has() there's nothing for the CPU to speculate wrongly.

And you _really_ want to get rid of that lfence, because currently you
pay that price even if you don't have to (!FEATURE_SPEC_CTRL).

> > And why are these boot_cpu_has() and not static_cpu_has() ?
> >
>
> It probably doesn't matter as we will be switching the check
> to the spec_ctrl_ibrs a couple of patches later.

Doesn't really matter does it, you can still use jump labels.