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

[Peter Zijlstra <peterz@infradead.org>]

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.