Re: [PATCH v4 2/2] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation

[Thomas Gleixner]

On Sat, 8 Sep 2018, Thomas Gleixner wrote:
> On Fri, 7 Sep 2018, Jiri Kosina wrote:
> > So I will go through the whole codepath again, but I fear your suggestion 
> > would not work -- see the check for cpu_smt_control in stibp_needed(). We 
> > need to see the old (or new, depending on the direction of the transition) 
> > value of cpu_smt_contol, which will break if we move 
> > arch_smt_enable_errata() (and thus the check).
> 
> That's bogus. The arch_smt_control(enable) function does not need to look
> at the SMT control state at all. The caller hands the not yet populated new
> state in and that's enough to make the decision whether to set or clear the
> bit in x86_spec_ctrl_base.

And thinking more about it, the function does not even need an argument.

smt_disable()

	res = shut_down_siblings();
	if (!res) {
		state = disabled;
		arch_smt_update();
	}

smt_enable()

	state = enabled;
	arch_smt_update();
	bringup_siblings();

So the update function has the correct state in both cases and then does:

	if (!cpu_is_trainwreck() || !cpu_has(stibp))
       		return;

	lock();
	ctrl = spec_ctrl_base;
	if (smt_disabled())
		ctrl &= ~STIPB;
	else
		ctrl |= STIPB;
	if (ctrl != spec_ctrl_base) {
		spec_ctrl_base = ctrl;
		on_each_cpu(write_spec_msr);
	}
	unlock();

See?

Thanks,

	tglx