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);
