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 */

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

Reply via email to