Re: [PATCH] x86/srso: fix build breakage for LD=ld.lld

[Fangrui Song]

On Wed, Aug 9, 2023 at 12:40 PM Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
>
> The assertion added to verify the difference in bits set of the
> addresses of srso_untrain_ret_alias and srso_safe_ret_alias would fail
> to link in LLVM's ld.lld linker with the following error:
>
>   ld.lld: error: ./arch/x86/kernel/vmlinux.lds:210: at least one side of
>   the expression must be absolute
>   ld.lld: error: ./arch/x86/kernel/vmlinux.lds:211: at least one side of
>   the expression must be absolute
>
> Use ABSOLUTE to evaluate the expression referring to at least one of the
> symbols so that LLD can evaluate the linker script.
>
> Also, add linker version info to the comment about xor being unsupported
> in either ld.bfd or ld.lld until somewhat recently.
>
> Fixes: fb3bd914b3ec ("x86/srso: Add a Speculative RAS Overflow mitigation")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1907
> Reported-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> Reported-by: Daniel Kolesa <daniel@xxxxxxxxxxxxx>
> Reported-by: Naresh Kamboju <naresh.kamboju@xxxxxxxxxx>
> Closes: https://lore.kernel.org/llvm/CA+G9fYsdUeNu-gwbs0+T6XHi4hYYk=Y9725-wFhZ7gJMspLDRA@xxxxxxxxxxxxxx/
> Suggested-by: Sven Volkinsfeld <thyrc@xxxxxxx>
> Signed-off-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> ---
> Note that CONFIG_LTO_CLANG is still broken due to GDS mitigations. Will
> work on a separate fix for that.  Sending this for now to unmuck the
> builds.
> ---
>  arch/x86/kernel/vmlinux.lds.S | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index e76813230192..ef06211bae4c 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -529,11 +529,17 @@ INIT_PER_CPU(irq_stack_backing_store);
>
>  #ifdef CONFIG_CPU_SRSO
>  /*
> - * GNU ld cannot do XOR so do: (A | B) - (A & B) in order to compute the XOR
> + * GNU ld cannot do XOR until 2.41.
> + * https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=f6f78318fca803c4907fb8d7f6ded8295f1947b1
> + *
> + * LLVM lld cannot do XOR until lld-17.
> + * https://github.com/llvm/llvm-project/commit/fae96104d4378166cbe5c875ef8ed808a356f3fb
> + *
> + * Instead do: (A | B) - (A & B) in order to compute the XOR
>   * of the two function addresses:
>   */
> -. = ASSERT(((srso_untrain_ret_alias | srso_safe_ret_alias) -
> -               (srso_untrain_ret_alias & srso_safe_ret_alias)) == ((1 << 2) | (1 << 8) | (1 << 14) | (1 << 20)),
> +. = ASSERT(((ABSOLUTE(srso_untrain_ret_alias) | srso_safe_ret_alias) -
> +               (ABSOLUTE(srso_untrain_ret_alias) & srso_safe_ret_alias)) == ((1 << 2) | (1 << 8) | (1 << 14) | (1 << 20)),
>                 "SRSO function pair won't alias");
>  #endif
>
>
> ---
> base-commit: 13b9372068660fe4f7023f43081067376582ef3c
> change-id: 20230809-gds-8b0456a18548
>
> Best regards,
> --
> Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
>
>

LGTM as a maintainer of lld/ELF and the author of the ^ ^= feature:)

Expressions in linker scripts are hard and GNU ld has quite a few
rules https://sourceware.org/binutils/docs/ld/Expression-Section.html
. I suspect the rule " Expressions appearing inside an output section
definition treat absolute symbols as numbers." is involved.

In any case, the semantics of the bitwise AND/OR results of two
addresses is unclear.
ld.lld reports "at least one side of the expression must be absolute",
which seems to right thing.

Reviewed-by: Fangrui Song <maskray@xxxxxxxxxx>


-- 
宋方睿