[lkml]   [2018]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH v2] x86/spectre_v1: Disable compiler optimizations over array_index_mask_nospec()
On Thu, Jun 7, 2018 at 9:23 AM Dan Williams <> wrote:
> Mark notes that gcc optimization passes have the potential to elide
> necessary invocations of this instruction sequence, so mark the asm
> volatile.

Ack. I'm not entirely sure this matters much, but it certainly isn't
wrong either.

The reason I'm not 100% convinced this matters is that gcc can *still*
mess things up for us by simply adding conditionals elsewhere.

For example, let's say we write this:

if (idx < foo) {
idx = array_idx_nospec(idx, foo);
} else {

then everything is obviously fine, right? With the volatile on the
array_idx_nospec(), we're guaranteed to use the right reduced idx, and
there's only one user, so we're all good.

Except maybe do_something(idx) looks something like this:

do_something(int idx)

and gcc decides that "hey, I can combine the two do_something_else()
cases", and then generates code that is basically

if (idx < foo)
idx = array_idx_nospec(idx, foo);
if (idx < foo)

instead. And now we're back to the "first branch can be predicted
correctly, second branch can be mis-predicted".

Honestly, I don't really care, and I don't think the kernel _should_
care. I don't think this is a problem in practice. I'm just saying
that adding a "volatile" on array_idx_nospec() doesn't really
guarantee anything, since it's not a volatile over the whole relevant
sequence, only over that small part.

So I think the volatile is fine, but I really think it doesn't matter
either. We're not going to plug every theoretical hole, and I think
the hole that the volatile plugs is theoretical, not practical.


 \ /
  Last update: 2018-06-07 19:07    [W:0.067 / U:3.628 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site