2022-08-17 19:08:15

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH] x86/build: Move '-mindirect-branch-cs-prefix' out of GCC-only block

LLVM 16 will have support for this flag so move it out of the GCC-only
block to allow LLVM builds to take advantage of it.

Link: https://github.com/ClangBuiltLinux/linux/issues/1665
Link: https://github.com/llvm/llvm-project/commit/6f867f9102838ebe314c1f3661fdf95700386e5a
Signed-off-by: Nathan Chancellor <[email protected]>
---

I was not sure if this information is relevant for the commit message
but I can boot without any issues on my test machines (two Intel and one
AMD).

arch/x86/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 7854685c5f25..987da87c7778 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -14,13 +14,13 @@ endif

ifdef CONFIG_CC_IS_GCC
RETPOLINE_CFLAGS := $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
-RETPOLINE_CFLAGS += $(call cc-option,-mindirect-branch-cs-prefix)
RETPOLINE_VDSO_CFLAGS := $(call cc-option,-mindirect-branch=thunk-inline -mindirect-branch-register)
endif
ifdef CONFIG_CC_IS_CLANG
RETPOLINE_CFLAGS := -mretpoline-external-thunk
RETPOLINE_VDSO_CFLAGS := -mretpoline
endif
+RETPOLINE_CFLAGS += $(call cc-option,-mindirect-branch-cs-prefix)

ifdef CONFIG_RETHUNK
RETHUNK_CFLAGS := -mfunction-return=thunk-extern

base-commit: 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
--
2.37.2


2022-08-17 22:22:08

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] x86/build: Move '-mindirect-branch-cs-prefix' out of GCC-only block

On Wed, Aug 17, 2022 at 11:56 AM Nathan Chancellor <[email protected]> wrote:
>
> LLVM 16 will have support for this flag so move it out of the GCC-only
> block to allow LLVM builds to take advantage of it.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1665
> Link: https://github.com/llvm/llvm-project/commit/6f867f9102838ebe314c1f3661fdf95700386e5a
> Signed-off-by: Nathan Chancellor <[email protected]>

Thanks for the patch, I also boot tested this (in virtual guests for
both 64b and 32b).

Reviewed-by: Nick Desaulniers <[email protected]>
Tested-by: Nick Desaulniers <[email protected]>

Peter, is there anything else special about these prefixes needed to
make use of them?

> ---
>
> I was not sure if this information is relevant for the commit message
> but I can boot without any issues on my test machines (two Intel and one
> AMD).
>
> arch/x86/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 7854685c5f25..987da87c7778 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -14,13 +14,13 @@ endif
>
> ifdef CONFIG_CC_IS_GCC
> RETPOLINE_CFLAGS := $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
> -RETPOLINE_CFLAGS += $(call cc-option,-mindirect-branch-cs-prefix)
> RETPOLINE_VDSO_CFLAGS := $(call cc-option,-mindirect-branch=thunk-inline -mindirect-branch-register)
> endif
> ifdef CONFIG_CC_IS_CLANG
> RETPOLINE_CFLAGS := -mretpoline-external-thunk
> RETPOLINE_VDSO_CFLAGS := -mretpoline
> endif
> +RETPOLINE_CFLAGS += $(call cc-option,-mindirect-branch-cs-prefix)
>
> ifdef CONFIG_RETHUNK
> RETHUNK_CFLAGS := -mfunction-return=thunk-extern
>
> base-commit: 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
> --
> 2.37.2
>
>


--
Thanks,
~Nick Desaulniers

2022-08-18 08:08:26

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] x86/build: Move '-mindirect-branch-cs-prefix' out of GCC-only block

On Wed, Aug 17, 2022 at 11:54:11AM -0700, Nathan Chancellor wrote:
> LLVM 16 will have support for this flag so move it out of the GCC-only
> block to allow LLVM builds to take advantage of it.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1665
> Link: https://github.com/llvm/llvm-project/commit/6f867f9102838ebe314c1f3661fdf95700386e5a
> Signed-off-by: Nathan Chancellor <[email protected]>

Acked-by: Peter Zijlstra (Intel) <[email protected]>

> ---
>
> I was not sure if this information is relevant for the commit message
> but I can boot without any issues on my test machines (two Intel and one
> AMD).
>
> arch/x86/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 7854685c5f25..987da87c7778 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -14,13 +14,13 @@ endif
>
> ifdef CONFIG_CC_IS_GCC
> RETPOLINE_CFLAGS := $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
> -RETPOLINE_CFLAGS += $(call cc-option,-mindirect-branch-cs-prefix)
> RETPOLINE_VDSO_CFLAGS := $(call cc-option,-mindirect-branch=thunk-inline -mindirect-branch-register)
> endif
> ifdef CONFIG_CC_IS_CLANG
> RETPOLINE_CFLAGS := -mretpoline-external-thunk
> RETPOLINE_VDSO_CFLAGS := -mretpoline
> endif
> +RETPOLINE_CFLAGS += $(call cc-option,-mindirect-branch-cs-prefix)

Thanks for using the same option!

2022-08-18 08:09:31

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] x86/build: Move '-mindirect-branch-cs-prefix' out of GCC-only block

On Wed, Aug 17, 2022 at 03:08:01PM -0700, Nick Desaulniers wrote:
> On Wed, Aug 17, 2022 at 11:56 AM Nathan Chancellor <[email protected]> wrote:
> >
> > LLVM 16 will have support for this flag so move it out of the GCC-only
> > block to allow LLVM builds to take advantage of it.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1665
> > Link: https://github.com/llvm/llvm-project/commit/6f867f9102838ebe314c1f3661fdf95700386e5a
> > Signed-off-by: Nathan Chancellor <[email protected]>
>
> Thanks for the patch, I also boot tested this (in virtual guests for
> both 64b and 32b).
>
> Reviewed-by: Nick Desaulniers <[email protected]>
> Tested-by: Nick Desaulniers <[email protected]>
>
> Peter, is there anything else special about these prefixes needed to
> make use of them?

The thing to do is boot with (warning, *very* verbose):

"spectre_v2=retpoline,lfence debug-alternative"

and observe that the retpoline sites all replace:

"cs call __x86_indirect_thunk_r11" (6 bytes)

with:

"lfence; jmp *%r11" (6 bytes)


This being clang, you'll ofcourse still have a few weird:

"Jcc.d32 __x86_indirect_thunk_r11"

sites that will not be able to be replaced, but there's nothing I can do
about that other than to continue to encourage y'all to stop emitting
them ;-)

2022-08-18 08:09:46

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] x86/build: Move '-mindirect-branch-cs-prefix' out of GCC-only block

On Thu, Aug 18, 2022 at 09:45:39AM +0200, Peter Zijlstra wrote:

> The thing to do is boot with (warning, *very* verbose):
>
> "spectre_v2=retpoline,lfence debug-alternative"
>
> and observe that the retpoline sites all replace:
>
> "cs call __x86_indirect_thunk_r11" (6 bytes)
>
> with:
>
> "lfence; jmp *%r11" (6 bytes)

^^ call, ofc, unless the original was jmp then jmp...

/me goes get more morning juice in an attempt to wake up.

>
>
> This being clang, you'll ofcourse still have a few weird:
>
> "Jcc.d32 __x86_indirect_thunk_r11"
>
> sites that will not be able to be replaced, but there's nothing I can do
> about that other than to continue to encourage y'all to stop emitting
> them ;-)

2022-09-30 18:20:59

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] x86/build: Move '-mindirect-branch-cs-prefix' out of GCC-only block

On Thu, Aug 18, 2022 at 12:45 AM Peter Zijlstra <[email protected]> wrote:
>
> This being clang, you'll ofcourse still have a few weird:
>
> "Jcc.d32 __x86_indirect_thunk_r11"
>
> sites that will not be able to be replaced, but there's nothing I can do
> about that other than to continue to encourage y'all to stop emitting
> them ;-)

Joao has a patch up for this:
https://reviews.llvm.org/D134915
--
Thanks,
~Nick Desaulniers

2022-10-04 21:36:21

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] x86/build: Move '-mindirect-branch-cs-prefix' out of GCC-only block

On Thu, Aug 18, 2022 at 12:45 AM Peter Zijlstra <[email protected]> wrote:
>
> On Wed, Aug 17, 2022 at 03:08:01PM -0700, Nick Desaulniers wrote:
> > On Wed, Aug 17, 2022 at 11:56 AM Nathan Chancellor <[email protected]> wrote:
> > >
> > > LLVM 16 will have support for this flag so move it out of the GCC-only
> > > block to allow LLVM builds to take advantage of it.
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1665
> > > Link: https://github.com/llvm/llvm-project/commit/6f867f9102838ebe314c1f3661fdf95700386e5a
> > > Signed-off-by: Nathan Chancellor <[email protected]>
> >
> > Thanks for the patch, I also boot tested this (in virtual guests for
> > both 64b and 32b).
> >
> > Reviewed-by: Nick Desaulniers <[email protected]>
> > Tested-by: Nick Desaulniers <[email protected]>
> >
> > Peter, is there anything else special about these prefixes needed to
> > make use of them?
>
> The thing to do is boot with (warning, *very* verbose):
>
> "spectre_v2=retpoline,lfence debug-alternative"

ah, debug-alternative also needs loglevel=8. Was wondering why I
wasn't getting anything new in the output...

No kidding on the verbosity. For a defconfig, booting in QEMU and
redirecting stdout to a file produced a 95257 line file.

$ grep "SMP alternatives: retpoline" /tmp/log.txt | wc -l
18623
$ du -h /tmp/log.txt
11M /tmp/log.txt
$ grep "SMP alternatives: retpoline" /tmp/log.txt | tr -s ' '
...
[ 56.736261] SMP alternatives: retpoline at:
pm_check_save_msr+0x30/0x80 (ffffffffaa34b320) len: 6 to:
__x86_indirect_thunk_r11+0x0/0x20
$ grep "SMP alternatives: retpoline" /tmp/log.txt | tr -s ' ' | cut -d
' ' -f 10 | sort -u
5
6
# Curious about the 5, looks like there's only 2 cases:
$ grep "SMP alternatives: retpoline" /tmp/log.txt | grep 'len: 5'
[ 0.147261] SMP alternatives: retpoline at: ret_from_fork+0x1d/0x30
(ffffffffa9601dbd) len: 5 to: __x86_indirect_thunk_rbx+0x0/0x20
[ 2.274261] SMP alternatives: retpoline at: __efi_call+0x23/0x30
(ffffffffa9671663) len: 5 to: __x86_indirect_thunk_rdi+0x0/0x20


Is there anything else I can do to help verify Nathan and Joao's
patches? If not, would you mind Ack'ing this so the x86 maintainers
can pick it up?


>
> and observe that the retpoline sites all replace:
>
> "cs call __x86_indirect_thunk_r11" (6 bytes)
>
> with:
>
> "lfence; jmp *%r11" (6 bytes)
>
>
> This being clang, you'll ofcourse still have a few weird:
>
> "Jcc.d32 __x86_indirect_thunk_r11"
>
> sites that will not be able to be replaced, but there's nothing I can do
> about that other than to continue to encourage y'all to stop emitting
> them ;-)

Noting that Joao's patch was applied in my tests.
https://reviews.llvm.org/D134915
I plan to land Joao's patch imminently.
--
Thanks,
~Nick Desaulniers


Attachments:
log.txt (24.40 kB)

2022-10-06 18:18:12

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] x86/build: Move '-mindirect-branch-cs-prefix' out of GCC-only block

On Tue, Oct 4, 2022 at 2:27 PM Nick Desaulniers <[email protected]> wrote:
>
> On Thu, Aug 18, 2022 at 12:45 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Wed, Aug 17, 2022 at 03:08:01PM -0700, Nick Desaulniers wrote:
> > > On Wed, Aug 17, 2022 at 11:56 AM Nathan Chancellor <[email protected]> wrote:
> > > >
> > > > LLVM 16 will have support for this flag so move it out of the GCC-only
> > > > block to allow LLVM builds to take advantage of it.
> > > >
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/1665
> > > > Link: https://github.com/llvm/llvm-project/commit/6f867f9102838ebe314c1f3661fdf95700386e5a
> > > > Signed-off-by: Nathan Chancellor <[email protected]>
> > >
> > > Thanks for the patch, I also boot tested this (in virtual guests for
> > > both 64b and 32b).
> > >
> > > Reviewed-by: Nick Desaulniers <[email protected]>
> > > Tested-by: Nick Desaulniers <[email protected]>
> > >
> > > Peter, is there anything else special about these prefixes needed to
> > > make use of them?
> >
> > The thing to do is boot with (warning, *very* verbose):
> >
> > "spectre_v2=retpoline,lfence debug-alternative"
>
> ah, debug-alternative also needs loglevel=8. Was wondering why I
> wasn't getting anything new in the output...
>
> No kidding on the verbosity. For a defconfig, booting in QEMU and
> redirecting stdout to a file produced a 95257 line file.
>
> $ grep "SMP alternatives: retpoline" /tmp/log.txt | wc -l
> 18623
> $ du -h /tmp/log.txt
> 11M /tmp/log.txt
> $ grep "SMP alternatives: retpoline" /tmp/log.txt | tr -s ' '
> ...
> [ 56.736261] SMP alternatives: retpoline at:
> pm_check_save_msr+0x30/0x80 (ffffffffaa34b320) len: 6 to:
> __x86_indirect_thunk_r11+0x0/0x20
> $ grep "SMP alternatives: retpoline" /tmp/log.txt | tr -s ' ' | cut -d
> ' ' -f 10 | sort -u
> 5
> 6
> # Curious about the 5, looks like there's only 2 cases:
> $ grep "SMP alternatives: retpoline" /tmp/log.txt | grep 'len: 5'
> [ 0.147261] SMP alternatives: retpoline at: ret_from_fork+0x1d/0x30
> (ffffffffa9601dbd) len: 5 to: __x86_indirect_thunk_rbx+0x0/0x20
> [ 2.274261] SMP alternatives: retpoline at: __efi_call+0x23/0x30
> (ffffffffa9671663) len: 5 to: __x86_indirect_thunk_rdi+0x0/0x20

For the typical len 6 case:
[ 0.710345] SMP alternatives: ffffffffa2b6d1f2: orig: 2e e8 88 5f 09 ff
[ 0.711345] SMP alternatives: ffffffffa2b6d1f2: repl: 0f ae e8 41 ff d3
[ 0.712345] SMP alternatives: retpoline at: setup_arch+0x4b0/0x58c
(ffffffffa2b6d2ef) len: 6 to: __x86_indirect_thunk_r11+0x0/0x20

2e is the cs prefix, so it looks like this is working.

https://github.com/llvm/llvm-project/issues/58201
:^)

> Is there anything else I can do to help verify Nathan and Joao's
> patches? If not, would you mind Ack'ing this so the x86 maintainers
> can pick it up?

I see it's already been Acked, so NVM. This is good to go.

> Noting that Joao's patch was applied in my tests.
> https://reviews.llvm.org/D134915
> I plan to land Joao's patch imminently.

Merging.
--
Thanks,
~Nick Desaulniers