Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 14 Feb 2018 09:57:32 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Tim Chen <tim.c.chen@linux.intel.com>
Cc:     Ingo Molnar <mingo@kernel.org>, Dave Hansen <dave@sr71.net>,
        hpa@zytor.com, tglx@linutronix.de, torvalds@linux-foundation.org,
        linux-kernel@vger.kernel.org, dwmw@amazon.co.uk,
        linux-tip-commits@vger.kernel.org, Borislav Petkov <bp@alien8.de>,
        Arjan van de Ven <arjan@infradead.org>
Subject: Re: [tip:x86/pti] x86/speculation: Use IBRS if available before
 calling into firmware
Message-ID: <20180214085732.GJ25235@hirez.programming.kicks-ass.net>
References: <1518362359-1005-1-git-send-email-dwmw@amazon.co.uk>
 <tip-670c3e8da87fa4046a55077b1409cf250865a203@git.kernel.org>
 <20180212102211.cdrrqqd4hdw7xu5y@gmail.com>
 <a85c28b8-1255-3264-bb4a-89e85e3c6009@sr71.net>
 <20180212165835.GO25181@hirez.programming.kicks-ass.net>
 <20180213075540.3lkikkpgjoe6ocjk@gmail.com>
 <5c3ba123-abbe-f153-7b75-a89d31d25c72@linux.intel.com>
 <20180214085614.GT25181@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180214085614.GT25181@hirez.programming.kicks-ass.net>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Feb 14, 2018 at 09:56:14AM +0100, Peter Zijlstra wrote:
> On Tue, Feb 13, 2018 at 05:49:47PM -0800, Tim Chen wrote:
> 
> >  static inline void firmware_restrict_branch_speculation_start(void)
> >  {
> > +	if (this_cpu_inc_return(spec_ctrl_ibrs_fw_depth) == 1)
> > +		alternative_msr_write(MSR_IA32_SPEC_CTRL, SPEC_CTRL_IBRS,
> >  			      X86_FEATURE_USE_IBRS_FW);
> >  }
> >  
> >  static inline void firmware_restrict_branch_speculation_end(void)
> >  {
> > +	if (this_cpu_dec_return(spec_ctrl_ibrs_fw_depth) == 0)
> > +		alternative_msr_write(MSR_IA32_SPEC_CTRL, 0,
> > +				      X86_FEATURE_USE_IBRS_FW);
> >  }
> 
> 
> At the very least this must disable and re-enable preemption, such that
> we guarantee we inc/dec the same counter. ISTR some firmware calls (EFI)
> actually are preemptible so that wouldn't work.
> 
> Further, consider:
> 
> 	this_cpu_inc_return()		// 0->1
> 	<NMI>
> 		this_cpu_inc_return()	// 1->2
> 		call_broken_arse_firmware()
> 		this_cpu_dec_return()	// 2->1
> 	</NMI>
> 	wrmsr(SPEC_CTRL, IBRS);
> 
> 	/* from dodgy firmware crap */

s/from/more/

typing hard.

> 	this_cpu_dec_return()		// 1->0
> 	wrmsr(SPEC_CTRL, 0);