Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757938AbeAHWLc (ORCPT + 1 other); Mon, 8 Jan 2018 17:11:32 -0500 Received: from bombadil.infradead.org ([65.50.211.133]:35447 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757411AbeAHWLa (ORCPT ); Mon, 8 Jan 2018 17:11:30 -0500 Date: Mon, 8 Jan 2018 23:11:21 +0100 From: Peter Zijlstra To: Andi Kleen Cc: dwmw@amazon.co.uk, pjt@google.com, linux-kernel@vger.kernel.org, gregkh@linux-foundation.org, tim.c.chen@linux.intel.com, dave.hansen@intel.com, tglx@linutronix.de, luto@amacapital.net, Andi Kleen Subject: Re: [PATCH] x86/retpoline: Avoid return buffer underflows on context switch Message-ID: <20180108221121.GU29822@worktop.programming.kicks-ass.net> References: <20180108201531.6782-1-andi@firstfloor.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180108201531.6782-1-andi@firstfloor.org> User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Mon, Jan 08, 2018 at 12:15:31PM -0800, Andi Kleen wrote: > diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h > index b8c8eeacb4be..e84e231248c2 100644 > --- a/arch/x86/include/asm/nospec-branch.h > +++ b/arch/x86/include/asm/nospec-branch.h > @@ -53,6 +53,35 @@ > #endif > .endm > > +/* > + * We use 32-N: 32 is the max return buffer size, > + * but there should have been at a minimum two > + * controlled calls already: one into the kernel > + * from entry*.S and another into the function > + * containing this macro. So N=2, thus 30. > + */ > +#define NUM_BRANCHES_TO_FILL 30 > + > +/* > + * Fill the CPU return branch buffer to prevent > + * indirect branch prediction on underflow. > + * Caller should check for X86_FEATURE_SMEP and X86_FEATURE_RETPOLINE > + */ > +.macro FILL_RETURN_BUFFER > +#ifdef CONFIG_RETPOLINE > + .rept NUM_BRANCHES_TO_FILL > + call 1221f > + pause /* stop speculation */ > +1221: > + .endr > +#ifdef CONFIG_64BIT > + addq $8*NUM_BRANCHES_TO_FILL, %rsp > +#else > + addl $4*NUM_BRANCHES_TO_FILL, %esp > +#endif > +#endif > +.endm So pjt did alignment, a single unroll and per discussion earlier today (CET) or late last night (PST), he only does 16. Why is none of that done here? Also, can we pretty please stop using those retarded number labels, they make this stuff unreadable. Also, pause is unlikely to stop speculation, that comment doesn't make sense. Looking at PJT's version there used to be a speculation trap in there, but I can't see that here.