[RFC PATCH 1/2] arm64: jump_label: use more precise asm constraints

[Nick Desaulniers]

On Wed, Apr 27, 2022 at 10:13 AM Ard Biesheuvel <ardb at kernel.org> wrote:
>
> In order to set bit #0 of the struct static_key pointer in the the jump
> label struct, we currently cast the pointer to char[], and take the
> address of either the 0th or 1st array member, depending on the value of
> 'branch'. This works but creates problems with -fpie code generation:
> GCC complains about the constraint being unsatisfiable, and Clang
> miscompiles the code in a way that causes stability issues (immediate
> panic on 'attempt to kill init')

Any more info on the "miscompile?" Perhaps worth an upstream bug report?

>
> So instead, pass the struct static_key reference and the 'branch'
> immediate individually, in a way that satisfies both GCC and Clang (GCC
> wants the 'S' constraint, whereas Clang wants the 'i' constraint for
> argument %0)

Anything we can do to improve Clang's handling of S constraints?
https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints
>> An absolute symbolic address or a label reference

>
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> ---
>  arch/arm64/include/asm/jump_label.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
> index cea441b6aa5d..f741bbacf175 100644
> --- a/arch/arm64/include/asm/jump_label.h
> +++ b/arch/arm64/include/asm/jump_label.h
> @@ -23,9 +23,9 @@ static __always_inline bool arch_static_branch(struct static_key *key,
>                  "      .pushsection    __jump_table, \"aw\"    \n\t"
>                  "      .align          3                       \n\t"
>                  "      .long           1b - ., %l[l_yes] - .   \n\t"
> -                "      .quad           %c0 - .                 \n\t"
> +                "      .quad           %c0 - . + %1            \n\t"

If %1 is "i" constrained, then I think we can use the %c output
template for it as well?
https://gcc.gnu.org/onlinedocs/gccint/Output-Template.html

Is the expression clearer if we keep the `- .` at the end of each expression?

>                  "      .popsection                             \n\t"
> -                :  :  "i"(&((char *)key)[branch]) :  : l_yes);
> +                :  :  "Si"(key), "i"(branch) :  : l_yes);
>
>         return false;
>  l_yes:
> @@ -40,9 +40,9 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key,
>                  "      .pushsection    __jump_table, \"aw\"    \n\t"
>                  "      .align          3                       \n\t"
>                  "      .long           1b - ., %l[l_yes] - .   \n\t"
> -                "      .quad           %c0 - .                 \n\t"
> +                "      .quad           %c0 - . + %1            \n\t"
>                  "      .popsection                             \n\t"
> -                :  :  "i"(&((char *)key)[branch]) :  : l_yes);
> +                :  :  "Si"(key), "i"(branch) :  : l_yes);
>
>         return false;
>  l_yes:
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers