Re: [PATCH v2.1] x86/retpoline: Fill return stack buffer on vmexit

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Thu, Jan 11, 2018 at 02:28:32PM +0000, David Woodhouse wrote:
> On Thu, 2018-01-11 at 08:20 -0600, Josh Poimboeuf wrote:
> > 
> > This seems weird.  I liked v1 a lot better.  What's the problem with
> > patching in the whole thing?
> > 
> > Also, if you go back to v1, it should be an easy objtool fix, just add
> > ANNOTATE_NOSPEC_ALTERNATIVE in front of it.
> 
> The objection was that I was patching in a fairly long set of
> instructions. I confess I don't actually know why that's a problem, but
> once I looked at it I realised the alignment was broken again. Again,
> alignment in the altinstr section doesn't necessarily mean alignment
> when it's copied into place over the oldinstr.

I'd *much* rather have things be consistent, and put all the crap code
behind specific features, like the retpolines already do.  It makes the
source code easier to read, object code easier to read, and, ahem, makes
objtool's life a lot easier.

In fact, Boris mentioned on IRC that I could remove the
ANNOTATE_NOSPEC_ALTERNATIVE annotations, and instead have objtool detect
the nospec stuff by looking at the alternative CPU feature bit, which it
already knows how to do.  So it would be really helpful to guard all
that nasty stuff behind alternatives.

I may have missed previous discussions about alignment, but if we
*really* needed alignment, maybe that could be accomplished by splitting
the alternative into two alternatives, with an .align directive as the
original instruction for the second one.

But we shouldn't even worry about alignment unless there's a real and
measureable reason to do so.

> exporting it... and defining it to take *one* register rather than
> being a macro... and ditched that approach then ended up with what's in
> v2.

Instead of exporting it you could keep it in the header file and give it
a "noinline" annotation so that it's out-of-line but still static.

-- 
Josh