Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 13 Oct 2021 17:12:13 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Andrew Cooper <andrew.cooper3@citrix.com>
Cc:     x86@kernel.org, jpoimboe@redhat.com, linux-kernel@vger.kernel.org,
        alexei.starovoitov@gmail.com, ndesaulniers@google.com
Subject: Re: [PATCH 4/9] x86/alternative: Implement .retpoline_sites support
Message-ID: <YWb3TdmyPK7GwBP4@hirez.programming.kicks-ass.net>
References: <20211013122217.304265366@infradead.org>
 <20211013123645.002402102@infradead.org>
 <98fdd068-d1c3-1f8c-18b7-13c3909ed9ca@citrix.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <98fdd068-d1c3-1f8c-18b7-13c3909ed9ca@citrix.com>
Precedence: bulk

On Wed, Oct 13, 2021 at 03:38:27PM +0100, Andrew Cooper wrote:
> On 13/10/2021 13:22, Peter Zijlstra wrote:
> > +/*
> > + * Rewrite the compiler generated retpoline thunk calls.
> > + *
> > + * For spectre_v2=off (!X86_FEATURE_RETPOLINE), rewrite them into immediate
> > + * indirect instructions, avoiding the extra indirection.
> > + *
> > + * For example, convert:
> > + *
> > + *   CALL __x86_indirect_thunk_\reg
> > + *
> > + * into:
> > + *
> > + *   CALL *%\reg
> > + *
> > + */
> > +static int patch_retpoline(void *addr, struct insn *insn, u8 *bytes)
> > +{
> > +	void (*target)(void);
> > +	int reg, i = 0;
> > +
> > +	if (cpu_feature_enabled(X86_FEATURE_RETPOLINE))
> > +		return -1;
> > +
> > +	target = addr + insn->length + insn->immediate.value;
> > +	reg = (target - &__x86_indirect_thunk_rax) /
> > +	      (&__x86_indirect_thunk_rcx - &__x86_indirect_thunk_rax);
> 
> This is equal measures beautiful and terrifying.

Thanks! :-)

> Something around here really wants to BUG_ON(reg == 4), because
> literally nothing good can come from selecting %rsp.

Ack, I had to add rsp to get the offsets right, but indeed, if anything
ever selects that we're in trouble.

> Also, it might be a good idea (previous patch perhaps) to have some
> linker assertions to confirm that the symbols are laid out safely to do
> this calculation.

I was hoping that since all this is in .S it would be immune from crazy
things like a compiler and do as told. But I suppose carzy stuff like
LTO (or worse BOLT) can totaly wreck this still (then BOLT won't care
about linker script assertions either).

I'll see if I can come up with something.