linux-kernel - Re: [patch 3/3] x86/process: Optimize TIF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 15 Dec 2016 09:31:39 -0800
From:   Andy Lutomirski <luto@...capital.net>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Kyle Huey <khuey@...ehuey.com>,
        Andy Lutomirski <luto@...nel.org>
Subject: Re: [patch 3/3] x86/process: Optimize TIF_NOTSC switch

On Thu, Dec 15, 2016 at 8:44 AM, Thomas Gleixner <tglx@...utronix.de> wrote:
> Provide and use a toggle helper instead of doing it with a branch.
>
> x86_64:
> 3662       8505      16   12183    2f97 Before
> 3646       8505      16   12167    2f87 After
>
> i386:
> 5906       9388    1804   17098    42ca Before
> 5834       9324    1740   16898    4202 After
>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
>  arch/x86/include/asm/tlbflush.h |   10 ++++++++++
>  arch/x86/kernel/process.c       |   22 ++++------------------
>  2 files changed, 14 insertions(+), 18 deletions(-)
>
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -110,6 +110,16 @@ static inline void cr4_clear_bits(unsign
>         }
>  }
>
> +static inline void cr4_toggle_bits(unsigned long mask)
> +{
> +       unsigned long cr4;
> +
> +       cr4 = this_cpu_read(cpu_tlbstate.cr4);
> +       cr4 ^= mask;
> +       this_cpu_write(cpu_tlbstate.cr4, cr4);
> +       __write_cr4(cr4);
> +}

This scares me for the same reason as BTF, although this should at
least be less fragile.  But how about:

static inline void cr4_set_bit_to(unsigned long mask, bool set)
{
  ...
  cr4 &= ~mask;
  cr4 ^= (set << ilog2(mask));
  ...
}

This should generate code that's nearly as good.