[PATCH] x86, retpolines: entirely disable switch jump tables when retpolines are enabled

[PATCH] x86, retpolines: entirely disable switch jump tables when retpolines are enabled

[Daniel Borkmann]

Commit ce02ef06fcf7 ("x86, retpolines: Raise limit for generating indirect
calls from switch-case") raised the limit under retpolines to 20 switch
cases where gcc would only then start to emit jump tables, and therefore
effectively disabling the emission of slow indirect calls in this area.

After this has been brought to attention to gcc folks [0], Martin Liska
has then fixed gcc to align with clang by avoiding to generate switch jump
tables entirely under retpolines. This is taking effect in gcc starting
from stable version 8.4.0. Given kernel supports compilation with older
versions of gcc where the fix is not being available or backported anymore,
we need to keep the extra KBUILD_CFLAGS around for some time and generally
set the -fno-jump-tables to align with what more recent gcc is doing
automatically today.

More than 20 switch cases are not expected to be fast-path critical, but
it would still be good to align with gcc behavior for versions < 8.4.0 in
order to have consistency across supported gcc versions. vmlinux size is
slightly growing by 0.27% for older gcc. This flag is only set to work
around affected gcc, no change for clang.

  [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86952

Suggested-by: Martin Liska <mliska@suse.cz>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: H.J. Lu <hjl.tools@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Björn Töpel <bjorn.topel@intel.com>
Cc: Magnus Karlsson <magnus.karlsson@intel.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: David S. Miller <davem@davemloft.net>
---
 arch/x86/Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d8b9d8ca4f8..a587805c6687 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -219,8 +219,12 @@ ifdef CONFIG_RETPOLINE
   # Additionally, avoid generating expensive indirect jumps which
   # are subject to retpolines for small number of switch cases.
   # clang turns off jump table generation by default when under
-  # retpoline builds, however, gcc does not for x86.
-  KBUILD_CFLAGS += $(call cc-option,--param=case-values-threshold=20)
+  # retpoline builds, however, gcc does not for x86. This has
+  # only been fixed starting from gcc stable version 8.4.0 and
+  # onwards, but not for older ones. See gcc bug #86952.
+  ifndef CONFIG_CC_IS_CLANG
+    KBUILD_CFLAGS += $(call cc-option,-fno-jump-tables)
+  endif
 endif
 
 archscripts: scripts_basic
-- 
2.17.1

Re: [PATCH] x86, retpolines: entirely disable switch jump tables when retpolines are enabled

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 657 bytes --]

On Mon, 2019-03-25 at 14:56 +0100, Daniel Borkmann wrote:
> More than 20 switch cases are not expected to be fast-path critical, but
> it would still be good to align with gcc behavior for versions < 8.4.0 in
> order to have consistency across supported gcc versions. vmlinux size is
> slightly growing by 0.27% for older gcc. This flag is only set to work
> around affected gcc, no change for clang.

I note your final sentence doesn't actually say that clang doesn't have
this problem, and doesn't *need* this (or an equivalent) change.

It should say that (if it's true). And if it isn't true, then other
remedial action would be in order.


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

Re: [PATCH] x86, retpolines: entirely disable switch jump tables when retpolines are enabled

[Daniel Borkmann]

On 03/25/2019 03:28 PM, David Woodhouse wrote:
> On Mon, 2019-03-25 at 14:56 +0100, Daniel Borkmann wrote:
>> More than 20 switch cases are not expected to be fast-path critical, but
>> it would still be good to align with gcc behavior for versions < 8.4.0 in
>> order to have consistency across supported gcc versions. vmlinux size is
>> slightly growing by 0.27% for older gcc. This flag is only set to work
>> around affected gcc, no change for clang.
> 
> I note your final sentence doesn't actually say that clang doesn't have
> this problem, and doesn't *need* this (or an equivalent) change.
> 
> It should say that (if it's true). And if it isn't true, then other
> remedial action would be in order.

clang doesn't have this problem as analyzed back in ce02ef06fcf7 ("x86,
retpolines: Raise limit for generating indirect calls from switch-case").

I thought both here would make it quite clear, from this patch commit msg:

"After this has been brought to attention to gcc folks [0], Martin Liska
 has then fixed gcc to align with clang by avoiding to generate switch
 jump tables entirely under retpolines."

And the comment in the Makefile code:

  # Additionally, avoid generating expensive indirect jumps which
  # are subject to retpolines for small number of switch cases.
  # clang turns off jump table generation by default when under
  # retpoline builds, however, gcc does not for x86. This has
  # only been fixed starting from gcc stable version 8.4.0 and
  # onwards, but not for older ones. See gcc bug #86952.

Thanks,
Daniel

Re: [PATCH] x86, retpolines: entirely disable switch jump tables when retpolines are enabled

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1819 bytes --]

On Mon, 2019-03-25 at 15:37 +0100, Daniel Borkmann wrote:
> On 03/25/2019 03:28 PM, David Woodhouse wrote:
> > On Mon, 2019-03-25 at 14:56 +0100, Daniel Borkmann wrote:
> > > More than 20 switch cases are not expected to be fast-path critical, but
> > > it would still be good to align with gcc behavior for versions < 8.4.0 in
> > > order to have consistency across supported gcc versions. vmlinux size is
> > > slightly growing by 0.27% for older gcc. This flag is only set to work
> > > around affected gcc, no change for clang.
> > 
> > I note your final sentence doesn't actually say that clang doesn't have
> > this problem, and doesn't *need* this (or an equivalent) change.
> > 
> > It should say that (if it's true). And if it isn't true, then other
> > remedial action would be in order.
> 
> clang doesn't have this problem as analyzed back in ce02ef06fcf7 ("x86,
> retpolines: Raise limit for generating indirect calls from switch-case").
> 
> I thought both here would make it quite clear, from this patch commit msg:
> 
> "After this has been brought to attention to gcc folks [0], Martin Liska
>  has then fixed gcc to align with clang by avoiding to generate switch
>  jump tables entirely under retpolines."
> 
> And the comment in the Makefile code:
> 
>   # Additionally, avoid generating expensive indirect jumps which
>   # are subject to retpolines for small number of switch cases.
>   # clang turns off jump table generation by default when under
>   # retpoline builds, however, gcc does not for x86. This has
>   # only been fixed starting from gcc stable version 8.4.0 and
>   # onwards, but not for older ones. See gcc bug #86952.

Hm, yes. That ought to be perfectly sufficient for anyone who can
actually read and is paying enough attention.

Sorry :)

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

[tip:x86/urgent] x86/retpolines: Disable switch jump tables when retpolines are enabled

[tip-bot for Daniel Borkmann]

Commit-ID:  a9d57ef15cbe327fe54416dd194ee0ea66ae53a4
Gitweb:     https://git.kernel.org/tip/a9d57ef15cbe327fe54416dd194ee0ea66ae53a4
Author:     Daniel Borkmann <daniel@iogearbox.net>
AuthorDate: Mon, 25 Mar 2019 14:56:20 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 28 Mar 2019 13:39:48 +0100

x86/retpolines: Disable switch jump tables when retpolines are enabled

Commit ce02ef06fcf7 ("x86, retpolines: Raise limit for generating indirect
calls from switch-case") raised the limit under retpolines to 20 switch
cases where gcc would only then start to emit jump tables, and therefore
effectively disabling the emission of slow indirect calls in this area.

After this has been brought to attention to gcc folks [0], Martin Liska
has then fixed gcc to align with clang by avoiding to generate switch jump
tables entirely under retpolines. This is taking effect in gcc starting
from stable version 8.4.0. Given kernel supports compilation with older
versions of gcc where the fix is not being available or backported anymore,
we need to keep the extra KBUILD_CFLAGS around for some time and generally
set the -fno-jump-tables to align with what more recent gcc is doing
automatically today.

More than 20 switch cases are not expected to be fast-path critical, but
it would still be good to align with gcc behavior for versions < 8.4.0 in
order to have consistency across supported gcc versions. vmlinux size is
slightly growing by 0.27% for older gcc. This flag is only set to work
around affected gcc, no change for clang.

  [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86952

Suggested-by: Martin Liska <mliska@suse.cz>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Björn Töpel<bjorn.topel@intel.com>
Cc: Magnus Karlsson <magnus.karlsson@intel.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: H.J. Lu <hjl.tools@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: David S. Miller <davem@davemloft.net>
Link: https://lkml.kernel.org/r/20190325135620.14882-1-daniel@iogearbox.net
---
 arch/x86/Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d8b9d8ca4f8..a587805c6687 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -219,8 +219,12 @@ ifdef CONFIG_RETPOLINE
   # Additionally, avoid generating expensive indirect jumps which
   # are subject to retpolines for small number of switch cases.
   # clang turns off jump table generation by default when under
-  # retpoline builds, however, gcc does not for x86.
-  KBUILD_CFLAGS += $(call cc-option,--param=case-values-threshold=20)
+  # retpoline builds, however, gcc does not for x86. This has
+  # only been fixed starting from gcc stable version 8.4.0 and
+  # onwards, but not for older ones. See gcc bug #86952.
+  ifndef CONFIG_CC_IS_CLANG
+    KBUILD_CFLAGS += $(call cc-option,-fno-jump-tables)
+  endif
 endif
 
 archscripts: scripts_basic