On Tue, 2018-01-09 at 10:37 +0100, Peter Zijlstra wrote:
> On Mon, Jan 08, 2018 at 03:51:26PM -0800, Andi Kleen wrote:
>
> >
> > @@ -107,8 +109,15 @@ static inline void mwait_idle_with_hints(unsigned long
> > eax, unsigned long ecx)
> > }
> >
> > __monitor((void *)¤t_thread_info()->flags, 0, 0);
> > - if (!need_resched())
> > + if (!need_resched()) {
> > __mwait(eax, ecx);
> > + /*
> > + * idle could have cleared the return buffer,
> > + * so fill it to prevent uncontrolled
> > + * speculation.
> > + */
> > + fill_return_buffer();
> wouldn't something like:
>
> if (static_cpu_has(X86_FEATURE_RETPOLINE))
> fill_return_buffer();
>
> be much saner? Then we avoid the entire call when not needed and you
> don't have to muck with the asm either.Hm... The background, of course, that that we need to be careful when doing things like this. If you end up with a conditional branch there, then processor can speculate right past it. There's a reason a lot of the IBRS-setting code has, effectively, an 'else lfence' in the cases where it isn't being done with ALTERNATIVEs. We had a *beautiful* case of that in the early IBRS patch set, on the syscall path, where the conditional branch opened up a path for speculative execution all the way to the jmp *sys_call_table(…). Now, as discussed on IRC, we can see that the current implementation of static_cpu_has using asm goto *is* generally doing the right thing and turning it into a straight unconditional jump over the fill_return_buffer() code. Clever GCC, have biscuit. However, you are suggesting that we turn the static_cpu_has() trick from a "nice to have" optimisation which is all very well when it pans out, to something we *rely* on for secure operation of the system. It never ends well when we rely on all versions of GCC optimising things precisely how we want. If you can build in a sanity check to ensure that the build will *fail* when GCC doesn't do what we want, I suppose we could live with that. But we don't have such a sanity check at the moment, do we?
smime.p7s
Description: S/MIME cryptographic signature
