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]
Message-ID: <CAKwvOdnCcpS_9A2y9tMqeiAg2NfcVx=gNeA2V=+zHknit7wGkg@mail.gmail.com>
Date:   Mon, 18 May 2020 14:15:44 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Brian Gerst <brgerst@...il.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 2/7] x86/percpu: Clean up percpu_to_op()

On Sun, May 17, 2020 at 8:29 AM Brian Gerst <brgerst@...il.com> wrote:
>
> The core percpu macros already have a switch on the data size, so the switch
> in the x86 code is redundant and produces more dead code.
>
> Also use appropriate types for the width of the instructions.  This avoids
> errors when compiling with Clang.
>
> Signed-off-by: Brian Gerst <brgerst@...il.com>
> ---
>  arch/x86/include/asm/percpu.h | 90 ++++++++++++++---------------------
>  1 file changed, 35 insertions(+), 55 deletions(-)
>
> diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
> index 89f918a3e99b..233c7a78d1a6 100644
> --- a/arch/x86/include/asm/percpu.h
> +++ b/arch/x86/include/asm/percpu.h
> @@ -117,37 +117,17 @@ extern void __bad_percpu_size(void);
>  #define __pcpu_reg_imm_4(x) "ri" (x)
>  #define __pcpu_reg_imm_8(x) "re" (x)
>
> -#define percpu_to_op(qual, op, var, val)               \
> -do {                                                   \
> -       typedef typeof(var) pto_T__;                    \
> -       if (0) {                                        \
> -               pto_T__ pto_tmp__;                      \
> -               pto_tmp__ = (val);                      \
> -               (void)pto_tmp__;                        \
> -       }                                               \
> -       switch (sizeof(var)) {                          \
> -       case 1:                                         \
> -               asm qual (op "b %1,"__percpu_arg(0)     \
> -                   : "+m" (var)                        \
> -                   : "qi" ((pto_T__)(val)));           \
> -               break;                                  \
> -       case 2:                                         \
> -               asm qual (op "w %1,"__percpu_arg(0)     \
> -                   : "+m" (var)                        \
> -                   : "ri" ((pto_T__)(val)));           \
> -               break;                                  \
> -       case 4:                                         \
> -               asm qual (op "l %1,"__percpu_arg(0)     \
> -                   : "+m" (var)                        \
> -                   : "ri" ((pto_T__)(val)));           \
> -               break;                                  \
> -       case 8:                                         \
> -               asm qual (op "q %1,"__percpu_arg(0)     \
> -                   : "+m" (var)                        \
> -                   : "re" ((pto_T__)(val)));           \
> -               break;                                  \
> -       default: __bad_percpu_size();                   \
> -       }                                               \
> +#define percpu_to_op(size, qual, op, _var, _val)                       \
> +do {                                                                   \
> +       __pcpu_type_##size pto_val__ = __pcpu_cast_##size(_val);        \
> +       if (0) {                                                        \
> +               typeof(_var) pto_tmp__;                                 \
> +               pto_tmp__ = (_val);                                     \
> +               (void)pto_tmp__;                                        \
> +       }                                                               \

Please replace the whole `if (0)` block with:
```c
__same_type(_var, _val);
```
from include/linux/compiler.h.

> +       asm qual(__pcpu_op2_##size(op, "%[val]", __percpu_arg([var]))   \
> +           : [var] "+m" (_var)                                         \
> +           : [val] __pcpu_reg_imm_##size(pto_val__));                  \
>  } while (0)
>
>  /*
> @@ -425,18 +405,18 @@ do {                                                                      \
>  #define raw_cpu_read_2(pcp)            percpu_from_op(, "mov", pcp)
>  #define raw_cpu_read_4(pcp)            percpu_from_op(, "mov", pcp)
>
> -#define raw_cpu_write_1(pcp, val)      percpu_to_op(, "mov", (pcp), val)
> -#define raw_cpu_write_2(pcp, val)      percpu_to_op(, "mov", (pcp), val)
> -#define raw_cpu_write_4(pcp, val)      percpu_to_op(, "mov", (pcp), val)
> +#define raw_cpu_write_1(pcp, val)      percpu_to_op(1, , "mov", (pcp), val)
> +#define raw_cpu_write_2(pcp, val)      percpu_to_op(2, , "mov", (pcp), val)
> +#define raw_cpu_write_4(pcp, val)      percpu_to_op(4, , "mov", (pcp), val)
>  #define raw_cpu_add_1(pcp, val)                percpu_add_op(, (pcp), val)
>  #define raw_cpu_add_2(pcp, val)                percpu_add_op(, (pcp), val)
>  #define raw_cpu_add_4(pcp, val)                percpu_add_op(, (pcp), val)
> -#define raw_cpu_and_1(pcp, val)                percpu_to_op(, "and", (pcp), val)
> -#define raw_cpu_and_2(pcp, val)                percpu_to_op(, "and", (pcp), val)
> -#define raw_cpu_and_4(pcp, val)                percpu_to_op(, "and", (pcp), val)
> -#define raw_cpu_or_1(pcp, val)         percpu_to_op(, "or", (pcp), val)
> -#define raw_cpu_or_2(pcp, val)         percpu_to_op(, "or", (pcp), val)
> -#define raw_cpu_or_4(pcp, val)         percpu_to_op(, "or", (pcp), val)
> +#define raw_cpu_and_1(pcp, val)                percpu_to_op(1, , "and", (pcp), val)
> +#define raw_cpu_and_2(pcp, val)                percpu_to_op(2, , "and", (pcp), val)
> +#define raw_cpu_and_4(pcp, val)                percpu_to_op(4, , "and", (pcp), val)
> +#define raw_cpu_or_1(pcp, val)         percpu_to_op(1, , "or", (pcp), val)
> +#define raw_cpu_or_2(pcp, val)         percpu_to_op(2, , "or", (pcp), val)
> +#define raw_cpu_or_4(pcp, val)         percpu_to_op(4, , "or", (pcp), val)
>
>  /*
>   * raw_cpu_xchg() can use a load-store since it is not required to be
> @@ -456,18 +436,18 @@ do {                                                                      \
>  #define this_cpu_read_1(pcp)           percpu_from_op(volatile, "mov", pcp)
>  #define this_cpu_read_2(pcp)           percpu_from_op(volatile, "mov", pcp)
>  #define this_cpu_read_4(pcp)           percpu_from_op(volatile, "mov", pcp)
> -#define this_cpu_write_1(pcp, val)     percpu_to_op(volatile, "mov", (pcp), val)
> -#define this_cpu_write_2(pcp, val)     percpu_to_op(volatile, "mov", (pcp), val)
> -#define this_cpu_write_4(pcp, val)     percpu_to_op(volatile, "mov", (pcp), val)
> +#define this_cpu_write_1(pcp, val)     percpu_to_op(1, volatile, "mov", (pcp), val)
> +#define this_cpu_write_2(pcp, val)     percpu_to_op(2, volatile, "mov", (pcp), val)
> +#define this_cpu_write_4(pcp, val)     percpu_to_op(4, volatile, "mov", (pcp), val)
>  #define this_cpu_add_1(pcp, val)       percpu_add_op(volatile, (pcp), val)
>  #define this_cpu_add_2(pcp, val)       percpu_add_op(volatile, (pcp), val)
>  #define this_cpu_add_4(pcp, val)       percpu_add_op(volatile, (pcp), val)
> -#define this_cpu_and_1(pcp, val)       percpu_to_op(volatile, "and", (pcp), val)
> -#define this_cpu_and_2(pcp, val)       percpu_to_op(volatile, "and", (pcp), val)
> -#define this_cpu_and_4(pcp, val)       percpu_to_op(volatile, "and", (pcp), val)
> -#define this_cpu_or_1(pcp, val)                percpu_to_op(volatile, "or", (pcp), val)
> -#define this_cpu_or_2(pcp, val)                percpu_to_op(volatile, "or", (pcp), val)
> -#define this_cpu_or_4(pcp, val)                percpu_to_op(volatile, "or", (pcp), val)
> +#define this_cpu_and_1(pcp, val)       percpu_to_op(1, volatile, "and", (pcp), val)
> +#define this_cpu_and_2(pcp, val)       percpu_to_op(2, volatile, "and", (pcp), val)
> +#define this_cpu_and_4(pcp, val)       percpu_to_op(4, volatile, "and", (pcp), val)
> +#define this_cpu_or_1(pcp, val)                percpu_to_op(1, volatile, "or", (pcp), val)
> +#define this_cpu_or_2(pcp, val)                percpu_to_op(2, volatile, "or", (pcp), val)
> +#define this_cpu_or_4(pcp, val)                percpu_to_op(4, volatile, "or", (pcp), val)
>  #define this_cpu_xchg_1(pcp, nval)     percpu_xchg_op(volatile, pcp, nval)
>  #define this_cpu_xchg_2(pcp, nval)     percpu_xchg_op(volatile, pcp, nval)
>  #define this_cpu_xchg_4(pcp, nval)     percpu_xchg_op(volatile, pcp, nval)
> @@ -509,19 +489,19 @@ do {                                                                      \
>   */
>  #ifdef CONFIG_X86_64
>  #define raw_cpu_read_8(pcp)                    percpu_from_op(, "mov", pcp)
> -#define raw_cpu_write_8(pcp, val)              percpu_to_op(, "mov", (pcp), val)
> +#define raw_cpu_write_8(pcp, val)              percpu_to_op(8, , "mov", (pcp), val)
>  #define raw_cpu_add_8(pcp, val)                        percpu_add_op(, (pcp), val)
> -#define raw_cpu_and_8(pcp, val)                        percpu_to_op(, "and", (pcp), val)
> -#define raw_cpu_or_8(pcp, val)                 percpu_to_op(, "or", (pcp), val)
> +#define raw_cpu_and_8(pcp, val)                        percpu_to_op(8, , "and", (pcp), val)
> +#define raw_cpu_or_8(pcp, val)                 percpu_to_op(8, , "or", (pcp), val)
>  #define raw_cpu_add_return_8(pcp, val)         percpu_add_return_op(, pcp, val)
>  #define raw_cpu_xchg_8(pcp, nval)              raw_percpu_xchg_op(pcp, nval)
>  #define raw_cpu_cmpxchg_8(pcp, oval, nval)     percpu_cmpxchg_op(, pcp, oval, nval)
>
>  #define this_cpu_read_8(pcp)                   percpu_from_op(volatile, "mov", pcp)
> -#define this_cpu_write_8(pcp, val)             percpu_to_op(volatile, "mov", (pcp), val)
> +#define this_cpu_write_8(pcp, val)             percpu_to_op(8, volatile, "mov", (pcp), val)
>  #define this_cpu_add_8(pcp, val)               percpu_add_op(volatile, (pcp), val)
> -#define this_cpu_and_8(pcp, val)               percpu_to_op(volatile, "and", (pcp), val)
> -#define this_cpu_or_8(pcp, val)                        percpu_to_op(volatile, "or", (pcp), val)
> +#define this_cpu_and_8(pcp, val)               percpu_to_op(8, volatile, "and", (pcp), val)
> +#define this_cpu_or_8(pcp, val)                        percpu_to_op(8, volatile, "or", (pcp), val)
>  #define this_cpu_add_return_8(pcp, val)                percpu_add_return_op(volatile, pcp, val)
>  #define this_cpu_xchg_8(pcp, nval)             percpu_xchg_op(volatile, pcp, nval)
>  #define this_cpu_cmpxchg_8(pcp, oval, nval)    percpu_cmpxchg_op(volatile, pcp, oval, nval)
> --
> 2.25.4
>


-- 
Thanks,
~Nick Desaulniers

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ