LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] cpu: Fix default mitigation behavior
@ 2024-04-17  0:15 Sean Christopherson
  2024-04-17  0:15 ` [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures Sean Christopherson
  2024-04-17  0:15 ` [PATCH 2/2] cpu: Ignore "mitigations" kernel parameter if CPU_MITIGATIONS=n Sean Christopherson
  0 siblings, 2 replies; 13+ messages in thread
From: Sean Christopherson @ 2024-04-17  0:15 UTC (permalink / raw
  To: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Greg Kroah-Hartman, Peter Zijlstra
  Cc: linux-doc, linux-kernel, Stephen Rothwell, Michael Ellerman,
	Geert Uytterhoeven, Sean Christopherson

This is effectively v2 of a previous series[*] that was intended to be
x86-only, but accidentally disabled CPU mitigations by default for every
other architectures.  Unfortunately, the buggy code has already made it's
way to Linus' tree.

Patch 1 fixes that goof by adding a generic Kconfig to control the
default behavior.

Patch 2 disallows retroactively enabling mitigations via command line if
the kernel was built with CPU_MITIGATIONS=n, i.e. with
SPECULATION_MITIGATIONS=n on x86, as it's infeasible for the kernel to
provide sane, predictable behavior for this scenario.

[*] https://lore.kernel.org/all/20240409175108.1512861-1-seanjc@google.com

Sean Christopherson (2):
  cpu: Re-enable CPU mitigations by default for !X86 architectures
  cpu: Ignore "mitigations" kernel parameter if CPU_MITIGATIONS=n

 Documentation/admin-guide/kernel-parameters.txt |  3 +++
 arch/x86/Kconfig                                | 11 ++++++++---
 drivers/base/Kconfig                            |  3 +++
 kernel/cpu.c                                    |  6 ++++--
 4 files changed, 18 insertions(+), 5 deletions(-)


base-commit: 96fca68c4fbf77a8185eb10f7557e23352732ea2
-- 
2.44.0.683.g7961c838ac-goog


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures
  2024-04-17  0:15 [PATCH 0/2] cpu: Fix default mitigation behavior Sean Christopherson
@ 2024-04-17  0:15 ` Sean Christopherson
  2024-04-18  0:54   ` Michael Ellerman
                     ` (3 more replies)
  2024-04-17  0:15 ` [PATCH 2/2] cpu: Ignore "mitigations" kernel parameter if CPU_MITIGATIONS=n Sean Christopherson
  1 sibling, 4 replies; 13+ messages in thread
From: Sean Christopherson @ 2024-04-17  0:15 UTC (permalink / raw
  To: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Greg Kroah-Hartman, Peter Zijlstra
  Cc: linux-doc, linux-kernel, Stephen Rothwell, Michael Ellerman,
	Geert Uytterhoeven, Sean Christopherson

Add a generic Kconfig, CPU_MITIGATIONS, to control whether or not CPU
mitigations are enabled by default, and force it on for all architectures
except x86.  A recent commit to turn mitigations off by default if
SPECULATION_MITIGATIONS=n kinda sorta missed that "cpu_mitigations" is
completely generic, where as SPECULATION_MITIGATIONS is x86 specific.

Alternatively, SPECULATION_MITIGATIONS could simply be defined in common
code, but that creates weirdness for x86 because SPECULATION_MITIGATIONS
ends up being defined twice, and the default behavior would likely depend
on the arbitrary include order (if the two definitions diverged).

Ideally, CPU_MITIGATIONS would be unconditionally on by default for all
architectures, and manually turned off, but there is no way to unselect a
Kconfig.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Closes: https://lkml.kernel.org/r/20240413115324.53303a68%40canb.auug.org.au
Fixes: f337a6a21e2f ("x86/cpu: Actually turn off mitigations by default for SPECULATION_MITIGATIONS=n")
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/Kconfig     | 1 +
 drivers/base/Kconfig | 3 +++
 kernel/cpu.c         | 4 ++--
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4474bf32d0a4..a0eca6313276 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2490,6 +2490,7 @@ config PREFIX_SYMBOLS
 
 menuconfig SPECULATION_MITIGATIONS
 	bool "Mitigations for speculative execution vulnerabilities"
+	select CPU_MITIGATIONS
 	default y
 	help
 	  Say Y here to enable options which enable mitigations for
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 2b8fd6bb7da0..dab19f15fa57 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -191,6 +191,9 @@ config GENERIC_CPU_AUTOPROBE
 config GENERIC_CPU_VULNERABILITIES
 	bool
 
+config CPU_MITIGATIONS
+	def_bool !X86
+
 config SOC_BUS
 	bool
 	select GLOB
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 07ad53b7f119..bb0ff275fb46 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -3207,8 +3207,8 @@ enum cpu_mitigations {
 };
 
 static enum cpu_mitigations cpu_mitigations __ro_after_init =
-	IS_ENABLED(CONFIG_SPECULATION_MITIGATIONS) ? CPU_MITIGATIONS_AUTO :
-						     CPU_MITIGATIONS_OFF;
+	IS_ENABLED(CONFIG_CPU_MITIGATIONS) ? CPU_MITIGATIONS_AUTO :
+					     CPU_MITIGATIONS_OFF;
 
 static int __init mitigations_parse_cmdline(char *arg)
 {
-- 
2.44.0.683.g7961c838ac-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] cpu: Ignore "mitigations" kernel parameter if CPU_MITIGATIONS=n
  2024-04-17  0:15 [PATCH 0/2] cpu: Fix default mitigation behavior Sean Christopherson
  2024-04-17  0:15 ` [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures Sean Christopherson
@ 2024-04-17  0:15 ` Sean Christopherson
  2024-04-19 15:00   ` Borislav Petkov
  1 sibling, 1 reply; 13+ messages in thread
From: Sean Christopherson @ 2024-04-17  0:15 UTC (permalink / raw
  To: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Greg Kroah-Hartman, Peter Zijlstra
  Cc: linux-doc, linux-kernel, Stephen Rothwell, Michael Ellerman,
	Geert Uytterhoeven, Sean Christopherson

Explicitly disallow enabling mitigations at runtime for kernels that were
built with CONFIG_CPU_MITIGATIONS=n, which currently is possible only on
x86 (via x86's SPECULATION_MITIGATIONS menuconfig).

On x86, a large pile of Kconfigs are buried behind SPECULATION_MITIGATIONS,
and trying to provide sane behavior for retroactively enabling mitigations
is extremely difficult, bordering on impossible.  E.g. page table isolation
and call depth tracking requrie build-time support, BHI mitigations will
still be off without additional kernel parameters, etc.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  3 +++
 arch/x86/Kconfig                                | 10 +++++++---
 kernel/cpu.c                                    |  2 ++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 902ecd92a29f..73cc672de9c3 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3423,6 +3423,9 @@
 			arch-independent options, each of which is an
 			aggregation of existing arch-specific options.
 
+			Note, "mitigations" is supported on x86 if and only if
+			the kernel was built with SPECULATION_MITIGATIONS=y.
+
 			off
 				Disable all optional CPU mitigations.  This
 				improves system performance, but it may also
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a0eca6313276..3021976e34cf 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2494,10 +2494,14 @@ menuconfig SPECULATION_MITIGATIONS
 	default y
 	help
 	  Say Y here to enable options which enable mitigations for
-	  speculative execution hardware vulnerabilities.
+	  speculative execution hardware vulnerabilities.  Mitigations can
+	  be disabled or restricted to SMT systems at runtime via the
+	  "mitigations" kernel parameter.
 
-	  If you say N, all mitigations will be disabled. You really
-	  should know what you are doing to say so.
+	  If you say N, all mitigations will be disabled.  This CANNOT be
+	  overridden at runtime.
+
+	  Say 'Y', unless you really know what you are doing.
 
 if SPECULATION_MITIGATIONS
 
diff --git a/kernel/cpu.c b/kernel/cpu.c
index bb0ff275fb46..e3f2b34bb378 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -3214,6 +3214,8 @@ static int __init mitigations_parse_cmdline(char *arg)
 {
 	if (!strcmp(arg, "off"))
 		cpu_mitigations = CPU_MITIGATIONS_OFF;
+	else if (!IS_ENABLED(CONFIG_CPU_MITIGATIONS))
+		pr_crit("Kernel compiled without mitigations, ignoring 'mitigations'; system may still be vulnerable\n");
 	else if (!strcmp(arg, "auto"))
 		cpu_mitigations = CPU_MITIGATIONS_AUTO;
 	else if (!strcmp(arg, "auto,nosmt"))
-- 
2.44.0.683.g7961c838ac-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures
  2024-04-17  0:15 ` [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures Sean Christopherson
@ 2024-04-18  0:54   ` Michael Ellerman
  2024-04-19 14:27   ` Geert Uytterhoeven
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Michael Ellerman @ 2024-04-18  0:54 UTC (permalink / raw
  To: Sean Christopherson, Jonathan Corbet, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	Greg Kroah-Hartman, Peter Zijlstra
  Cc: linux-doc, linux-kernel, Stephen Rothwell, Geert Uytterhoeven,
	Sean Christopherson

Sean Christopherson <seanjc@google.com> writes:
> Add a generic Kconfig, CPU_MITIGATIONS, to control whether or not CPU
> mitigations are enabled by default, and force it on for all architectures
> except x86.  A recent commit to turn mitigations off by default if
> SPECULATION_MITIGATIONS=n kinda sorta missed that "cpu_mitigations" is
> completely generic, where as SPECULATION_MITIGATIONS is x86 specific.
>
> Alternatively, SPECULATION_MITIGATIONS could simply be defined in common
> code, but that creates weirdness for x86 because SPECULATION_MITIGATIONS
> ends up being defined twice, and the default behavior would likely depend
> on the arbitrary include order (if the two definitions diverged).
>
> Ideally, CPU_MITIGATIONS would be unconditionally on by default for all
> architectures, and manually turned off, but there is no way to unselect a
> Kconfig.
>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Closes: https://lkml.kernel.org/r/20240413115324.53303a68%40canb.auug.org.au
> Fixes: f337a6a21e2f ("x86/cpu: Actually turn off mitigations by default for SPECULATION_MITIGATIONS=n")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---

Thanks for fixing it up.

Tested-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

cheers

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures
  2024-04-17  0:15 ` [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures Sean Christopherson
  2024-04-18  0:54   ` Michael Ellerman
@ 2024-04-19 14:27   ` Geert Uytterhoeven
  2024-04-19 14:37   ` Will Deacon
  2024-04-19 16:05   ` Josh Poimboeuf
  3 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2024-04-19 14:27 UTC (permalink / raw
  To: Sean Christopherson
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Greg Kroah-Hartman, Peter Zijlstra, linux-doc,
	linux-kernel, Stephen Rothwell, Michael Ellerman

On Wed, Apr 17, 2024 at 2:15 AM Sean Christopherson <seanjc@google.com> wrote:
> Add a generic Kconfig, CPU_MITIGATIONS, to control whether or not CPU
> mitigations are enabled by default, and force it on for all architectures
> except x86.  A recent commit to turn mitigations off by default if
> SPECULATION_MITIGATIONS=n kinda sorta missed that "cpu_mitigations" is
> completely generic, where as SPECULATION_MITIGATIONS is x86 specific.
>
> Alternatively, SPECULATION_MITIGATIONS could simply be defined in common
> code, but that creates weirdness for x86 because SPECULATION_MITIGATIONS
> ends up being defined twice, and the default behavior would likely depend
> on the arbitrary include order (if the two definitions diverged).
>
> Ideally, CPU_MITIGATIONS would be unconditionally on by default for all
> architectures, and manually turned off, but there is no way to unselect a
> Kconfig.
>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Closes: https://lkml.kernel.org/r/20240413115324.53303a68%40canb.auug.org.au
> Fixes: f337a6a21e2f ("x86/cpu: Actually turn off mitigations by default for SPECULATION_MITIGATIONS=n")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures
  2024-04-17  0:15 ` [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures Sean Christopherson
  2024-04-18  0:54   ` Michael Ellerman
  2024-04-19 14:27   ` Geert Uytterhoeven
@ 2024-04-19 14:37   ` Will Deacon
  2024-04-19 16:05   ` Josh Poimboeuf
  3 siblings, 0 replies; 13+ messages in thread
From: Will Deacon @ 2024-04-19 14:37 UTC (permalink / raw
  To: Sean Christopherson
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Greg Kroah-Hartman, Peter Zijlstra, linux-doc,
	linux-kernel, Stephen Rothwell, Michael Ellerman,
	Geert Uytterhoeven

On Tue, Apr 16, 2024 at 05:15:06PM -0700, Sean Christopherson wrote:
> Add a generic Kconfig, CPU_MITIGATIONS, to control whether or not CPU
> mitigations are enabled by default, and force it on for all architectures
> except x86.  A recent commit to turn mitigations off by default if
> SPECULATION_MITIGATIONS=n kinda sorta missed that "cpu_mitigations" is
> completely generic, where as SPECULATION_MITIGATIONS is x86 specific.
> 
> Alternatively, SPECULATION_MITIGATIONS could simply be defined in common
> code, but that creates weirdness for x86 because SPECULATION_MITIGATIONS
> ends up being defined twice, and the default behavior would likely depend
> on the arbitrary include order (if the two definitions diverged).
> 
> Ideally, CPU_MITIGATIONS would be unconditionally on by default for all
> architectures, and manually turned off, but there is no way to unselect a
> Kconfig.
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Closes: https://lkml.kernel.org/r/20240413115324.53303a68%40canb.auug.org.au
> Fixes: f337a6a21e2f ("x86/cpu: Actually turn off mitigations by default for SPECULATION_MITIGATIONS=n")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/Kconfig     | 1 +
>  drivers/base/Kconfig | 3 +++
>  kernel/cpu.c         | 4 ++--
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 4474bf32d0a4..a0eca6313276 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2490,6 +2490,7 @@ config PREFIX_SYMBOLS
>  
>  menuconfig SPECULATION_MITIGATIONS
>  	bool "Mitigations for speculative execution vulnerabilities"
> +	select CPU_MITIGATIONS
>  	default y
>  	help
>  	  Say Y here to enable options which enable mitigations for
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 2b8fd6bb7da0..dab19f15fa57 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -191,6 +191,9 @@ config GENERIC_CPU_AUTOPROBE
>  config GENERIC_CPU_VULNERABILITIES
>  	bool
>  
> +config CPU_MITIGATIONS
> +	def_bool !X86
> +
>  config SOC_BUS
>  	bool
>  	select GLOB
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 07ad53b7f119..bb0ff275fb46 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -3207,8 +3207,8 @@ enum cpu_mitigations {
>  };
>  
>  static enum cpu_mitigations cpu_mitigations __ro_after_init =
> -	IS_ENABLED(CONFIG_SPECULATION_MITIGATIONS) ? CPU_MITIGATIONS_AUTO :
> -						     CPU_MITIGATIONS_OFF;
> +	IS_ENABLED(CONFIG_CPU_MITIGATIONS) ? CPU_MITIGATIONS_AUTO :
> +					     CPU_MITIGATIONS_OFF;
>  
>  static int __init mitigations_parse_cmdline(char *arg)
>  {
> -- 
> 2.44.0.683.g7961c838ac-goog

Thanks, Sean!

Acked-by: Will Deacon <will@kernel.org>

Will

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] cpu: Ignore "mitigations" kernel parameter if CPU_MITIGATIONS=n
  2024-04-17  0:15 ` [PATCH 2/2] cpu: Ignore "mitigations" kernel parameter if CPU_MITIGATIONS=n Sean Christopherson
@ 2024-04-19 15:00   ` Borislav Petkov
  2024-04-19 16:01     ` Sean Christopherson
  0 siblings, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2024-04-19 15:00 UTC (permalink / raw
  To: Sean Christopherson
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	Greg Kroah-Hartman, Peter Zijlstra, linux-doc, linux-kernel,
	Stephen Rothwell, Michael Ellerman, Geert Uytterhoeven

On Tue, Apr 16, 2024 at 05:15:07PM -0700, Sean Christopherson wrote:
> Explicitly disallow enabling mitigations at runtime for kernels that were
> built with CONFIG_CPU_MITIGATIONS=n, which currently is possible only on
> x86 (via x86's SPECULATION_MITIGATIONS menuconfig).

Hm, so the umbrella term is CPU_MITIGATIONS, the x86-one is
SPECULATION_MITIGATIONS.

I wanna streamline our namespacing and say, the arch agnostic term
should be CPU_MITIGATIONS and the x86 one should be then
X86_CPU_MITIGATIONS, the Arm one would be ARM_CPU_MITIGATIONS and so on.

This way we can stick all kinds of special mitigations code - not only
speculative execution ones - under those config items and have it all
straight from the get-go.

And I think we should do it now, before it all propagates down the tree
and becomes a lot harder to rename.

Thoughts?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] cpu: Ignore "mitigations" kernel parameter if CPU_MITIGATIONS=n
  2024-04-19 15:00   ` Borislav Petkov
@ 2024-04-19 16:01     ` Sean Christopherson
  0 siblings, 0 replies; 13+ messages in thread
From: Sean Christopherson @ 2024-04-19 16:01 UTC (permalink / raw
  To: Borislav Petkov
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	Greg Kroah-Hartman, Peter Zijlstra, linux-doc, linux-kernel,
	Stephen Rothwell, Michael Ellerman, Geert Uytterhoeven

On Fri, Apr 19, 2024, Borislav Petkov wrote:
> On Tue, Apr 16, 2024 at 05:15:07PM -0700, Sean Christopherson wrote:
> > Explicitly disallow enabling mitigations at runtime for kernels that were
> > built with CONFIG_CPU_MITIGATIONS=n, which currently is possible only on
> > x86 (via x86's SPECULATION_MITIGATIONS menuconfig).
> 
> Hm, so the umbrella term is CPU_MITIGATIONS, the x86-one is
> SPECULATION_MITIGATIONS.
> 
> I wanna streamline our namespacing and say, the arch agnostic term
> should be CPU_MITIGATIONS and the x86 one should be then
> X86_CPU_MITIGATIONS, the Arm one would be ARM_CPU_MITIGATIONS and so on.

+1.  That would help avoid goofs like mine.  Maybe.  :-)

> This way we can stick all kinds of special mitigations code - not only
> speculative execution ones - under those config items and have it all
> straight from the get-go.
> 
> And I think we should do it now, before it all propagates down the tree
> and becomes a lot harder to rename.
> 
> Thoughts?
> 
> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures
  2024-04-17  0:15 ` [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures Sean Christopherson
                     ` (2 preceding siblings ...)
  2024-04-19 14:37   ` Will Deacon
@ 2024-04-19 16:05   ` Josh Poimboeuf
  2024-04-19 16:46     ` Sean Christopherson
  2024-04-19 23:27     ` Michael Ellerman
  3 siblings, 2 replies; 13+ messages in thread
From: Josh Poimboeuf @ 2024-04-19 16:05 UTC (permalink / raw
  To: Sean Christopherson
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Greg Kroah-Hartman, Peter Zijlstra, linux-doc,
	linux-kernel, Stephen Rothwell, Michael Ellerman,
	Geert Uytterhoeven

On Tue, Apr 16, 2024 at 05:15:06PM -0700, Sean Christopherson wrote:
> Add a generic Kconfig, CPU_MITIGATIONS, to control whether or not CPU
> mitigations are enabled by default, and force it on for all architectures
> except x86.  A recent commit to turn mitigations off by default if
> SPECULATION_MITIGATIONS=n kinda sorta missed that "cpu_mitigations" is
> completely generic, where as SPECULATION_MITIGATIONS is x86 specific.
> 
> Alternatively, SPECULATION_MITIGATIONS could simply be defined in common
> code, but that creates weirdness for x86 because SPECULATION_MITIGATIONS
> ends up being defined twice, and the default behavior would likely depend
> on the arbitrary include order (if the two definitions diverged).
> 
> Ideally, CPU_MITIGATIONS would be unconditionally on by default for all
> architectures, and manually turned off, but there is no way to unselect a
> Kconfig.
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Closes: https://lkml.kernel.org/r/20240413115324.53303a68%40canb.auug.org.au
> Fixes: f337a6a21e2f ("x86/cpu: Actually turn off mitigations by default for SPECULATION_MITIGATIONS=n")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>

It seems confusing to have two config options which have very similar
names and similar purposes (with subtle differences depending on the
arch).

How about we instead just get rid of the x86-specific
SPECULATION_MITIGATIONS and replace it with a menu which depends on
CPU_MITIGATIONS:

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4474bf32d0a4..85a4d57bce1e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2488,17 +2488,8 @@ config PREFIX_SYMBOLS
 	def_bool y
 	depends on CALL_PADDING && !CFI_CLANG
 
-menuconfig SPECULATION_MITIGATIONS
-	bool "Mitigations for speculative execution vulnerabilities"
-	default y
-	help
-	  Say Y here to enable options which enable mitigations for
-	  speculative execution hardware vulnerabilities.
-
-	  If you say N, all mitigations will be disabled. You really
-	  should know what you are doing to say so.
-
-if SPECULATION_MITIGATIONS
+menu "CPU speculative execution mitigation defaults"
+	depends on CPU_MITIGATIONS
 
 config MITIGATION_PAGE_TABLE_ISOLATION
 	bool "Remove the kernel mapping in user mode"
@@ -2643,7 +2634,7 @@ config MITIGATION_SPECTRE_BHI
 	  indirect branches.
 	  See <file:Documentation/admin-guide/hw-vuln/spectre.rst>
 
-endif
+endmenu
 
 config ARCH_HAS_ADD_PAGES
 	def_bool y
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 2b8fd6bb7da0..70c1e7eb64f0 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -191,6 +191,16 @@ config GENERIC_CPU_AUTOPROBE
 config GENERIC_CPU_VULNERABILITIES
 	bool
 
+config CPU_MITIGATIONS
+	bool "Mitigations for CPU speculative execution vulnerabilities"
+	default y
+	help
+	  Say Y here to enable mitigations for CPU speculative execution
+	  vulnerabilities.
+
+	  If you say N, all mitigations will be disabled. You really
+	  should know what you are doing to say so.
+
 config SOC_BUS
 	bool
 	select GLOB
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 07ad53b7f119..bb0ff275fb46 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -3207,8 +3207,8 @@ enum cpu_mitigations {
 };
 
 static enum cpu_mitigations cpu_mitigations __ro_after_init =
-	IS_ENABLED(CONFIG_SPECULATION_MITIGATIONS) ? CPU_MITIGATIONS_AUTO :
-						     CPU_MITIGATIONS_OFF;
+	IS_ENABLED(CONFIG_CPU_MITIGATIONS) ? CPU_MITIGATIONS_AUTO :
+					     CPU_MITIGATIONS_OFF;
 
 static int __init mitigations_parse_cmdline(char *arg)
 {

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures
  2024-04-19 16:05   ` Josh Poimboeuf
@ 2024-04-19 16:46     ` Sean Christopherson
  2024-04-19 17:34       ` Josh Poimboeuf
  2024-04-19 23:27     ` Michael Ellerman
  1 sibling, 1 reply; 13+ messages in thread
From: Sean Christopherson @ 2024-04-19 16:46 UTC (permalink / raw
  To: Josh Poimboeuf
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Greg Kroah-Hartman, Peter Zijlstra, linux-doc,
	linux-kernel, Stephen Rothwell, Michael Ellerman,
	Geert Uytterhoeven

On Fri, Apr 19, 2024, Josh Poimboeuf wrote:
> On Tue, Apr 16, 2024 at 05:15:06PM -0700, Sean Christopherson wrote:
> > Add a generic Kconfig, CPU_MITIGATIONS, to control whether or not CPU
> > mitigations are enabled by default, and force it on for all architectures
> > except x86.  A recent commit to turn mitigations off by default if
> > SPECULATION_MITIGATIONS=n kinda sorta missed that "cpu_mitigations" is
> > completely generic, where as SPECULATION_MITIGATIONS is x86 specific.
> > 
> > Alternatively, SPECULATION_MITIGATIONS could simply be defined in common
> > code, but that creates weirdness for x86 because SPECULATION_MITIGATIONS
> > ends up being defined twice, and the default behavior would likely depend
> > on the arbitrary include order (if the two definitions diverged).
> > 
> > Ideally, CPU_MITIGATIONS would be unconditionally on by default for all
> > architectures, and manually turned off, but there is no way to unselect a
> > Kconfig.
> > 
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Reported-by: Michael Ellerman <mpe@ellerman.id.au>
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Closes: https://lkml.kernel.org/r/20240413115324.53303a68%40canb.auug.org.au
> > Fixes: f337a6a21e2f ("x86/cpu: Actually turn off mitigations by default for SPECULATION_MITIGATIONS=n")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> 
> It seems confusing to have two config options which have very similar
> names and similar purposes (with subtle differences depending on the
> arch).
> 
> How about we instead just get rid of the x86-specific
> SPECULATION_MITIGATIONS and replace it with a menu which depends on
> CPU_MITIGATIONS:

Huh, didn't realize that was possible.

I agree that having two things for the same thing is confusing, though Boris'
idea to do s/SPECULATION_MITIGATIONS/X86_CPU_MITIGATIONS would help a fair bit
on that front.

My only hesitation is that x86's menu and the common config knob end up in
completely different locations.  And AFAICT, the parser doesn't allow sourcing
menu entires from a different file:

  init/Kconfig:1959: 'menu' in different file than 'menu'

e.g. we can't declare the menuconfig in common code and then include arch
definitions.

Regardless of whether or not we shuffle things around, CPU_MITIGATIONS really
should be in init/Kconfig, not drivers/base/Kconfig, e.g. so that if we make it
a user-selectable option, it shows up under "General setup" instead of being
buried two layers deep in drivers.

That makes it less hard to find CPU_MITIGATIONS, but I still find it cumbersome
to have to enable CPU_MITIGATIONS, and then go hunting for x86's menu.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures
  2024-04-19 16:46     ` Sean Christopherson
@ 2024-04-19 17:34       ` Josh Poimboeuf
  2024-04-19 23:57         ` Sean Christopherson
  0 siblings, 1 reply; 13+ messages in thread
From: Josh Poimboeuf @ 2024-04-19 17:34 UTC (permalink / raw
  To: Sean Christopherson
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Greg Kroah-Hartman, Peter Zijlstra, linux-doc,
	linux-kernel, Stephen Rothwell, Michael Ellerman,
	Geert Uytterhoeven

On Fri, Apr 19, 2024 at 09:46:58AM -0700, Sean Christopherson wrote:
> > It seems confusing to have two config options which have very similar
> > names and similar purposes (with subtle differences depending on the
> > arch).
> > 
> > How about we instead just get rid of the x86-specific
> > SPECULATION_MITIGATIONS and replace it with a menu which depends on
> > CPU_MITIGATIONS:
> 
> Huh, didn't realize that was possible.
> 
> I agree that having two things for the same thing is confusing, though Boris'
> idea to do s/SPECULATION_MITIGATIONS/X86_CPU_MITIGATIONS would help a fair bit
> on that front.
> 
> My only hesitation is that x86's menu and the common config knob end up in
> completely different locations.

I'm thinking this is a minor issue because CPU_MITIGATIONS is enabled by
default, so it should almost always be enabled unless the user disables
it, in which case they wouldn't be looking for the x86-specific
mitigations anyway.

Regardless it seems very common for a menu "depends on" to be in a
different file.  We could put CPU_MITIGATIONS in arch/Kconfig which is a
fairly logical place for the dependency.

-- 
Josh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures
  2024-04-19 16:05   ` Josh Poimboeuf
  2024-04-19 16:46     ` Sean Christopherson
@ 2024-04-19 23:27     ` Michael Ellerman
  1 sibling, 0 replies; 13+ messages in thread
From: Michael Ellerman @ 2024-04-19 23:27 UTC (permalink / raw
  To: Josh Poimboeuf, Sean Christopherson
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Greg Kroah-Hartman, Peter Zijlstra, linux-doc,
	linux-kernel, Stephen Rothwell, Geert Uytterhoeven

Josh Poimboeuf <jpoimboe@kernel.org> writes:
> On Tue, Apr 16, 2024 at 05:15:06PM -0700, Sean Christopherson wrote:
>> Add a generic Kconfig, CPU_MITIGATIONS, to control whether or not CPU
>> mitigations are enabled by default, and force it on for all architectures
>> except x86.  A recent commit to turn mitigations off by default if
>> SPECULATION_MITIGATIONS=n kinda sorta missed that "cpu_mitigations" is
>> completely generic, where as SPECULATION_MITIGATIONS is x86 specific.
>> 
>> Alternatively, SPECULATION_MITIGATIONS could simply be defined in common
>> code, but that creates weirdness for x86 because SPECULATION_MITIGATIONS
>> ends up being defined twice, and the default behavior would likely depend
>> on the arbitrary include order (if the two definitions diverged).
>> 
>> Ideally, CPU_MITIGATIONS would be unconditionally on by default for all
>> architectures, and manually turned off, but there is no way to unselect a
>> Kconfig.
>> 
>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Closes: https://lkml.kernel.org/r/20240413115324.53303a68%40canb.auug.org.au
>> Fixes: f337a6a21e2f ("x86/cpu: Actually turn off mitigations by default for SPECULATION_MITIGATIONS=n")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>
> It seems confusing to have two config options which have very similar
> names and similar purposes (with subtle differences depending on the
> arch).

I agree.

But can we please get Sean's fix into mainline before rc5.

cheers

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures
  2024-04-19 17:34       ` Josh Poimboeuf
@ 2024-04-19 23:57         ` Sean Christopherson
  0 siblings, 0 replies; 13+ messages in thread
From: Sean Christopherson @ 2024-04-19 23:57 UTC (permalink / raw
  To: Josh Poimboeuf
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Greg Kroah-Hartman, Peter Zijlstra, linux-doc,
	linux-kernel, Stephen Rothwell, Michael Ellerman,
	Geert Uytterhoeven

On Fri, Apr 19, 2024, Josh Poimboeuf wrote:
> On Fri, Apr 19, 2024 at 09:46:58AM -0700, Sean Christopherson wrote:
> > > It seems confusing to have two config options which have very similar
> > > names and similar purposes (with subtle differences depending on the
> > > arch).
> > > 
> > > How about we instead just get rid of the x86-specific
> > > SPECULATION_MITIGATIONS and replace it with a menu which depends on
> > > CPU_MITIGATIONS:
> > 
> > Huh, didn't realize that was possible.
> > 
> > I agree that having two things for the same thing is confusing, though Boris'
> > idea to do s/SPECULATION_MITIGATIONS/X86_CPU_MITIGATIONS would help a fair bit
> > on that front.
> > 
> > My only hesitation is that x86's menu and the common config knob end up in
> > completely different locations.
> 
> I'm thinking this is a minor issue because CPU_MITIGATIONS is enabled by
> default, so it should almost always be enabled unless the user disables
> it, in which case they wouldn't be looking for the x86-specific
> mitigations anyway.

Yeah, this isn't a sticking point by any means.

Oh, and another hiccup I almost forgot about (I just recalled Geert's report).
Letting CPU_MITIGATIONS be disabled for every arch at compile time will obsolete
a small amount of kernel code, e.g. arm64 explicitly says "disabled by command
line option" in a few places.

Those are easy enough to fixup though, but it's not clear that other architectures
*want* to allow mitigations to be completely compiled out.  x86 appears to be
relatively unique in that it has a bajillion different things being mitigated.

Rather than making CPU_MITIGATIONS configured for all architectures, what if
use another Kconfig to tell common code that arch code has already defined
CPU_MITIGATIONS?  The big downside is that if another arch does end up letting
the user disable CPU_MITIGATIONS, then we'll probably end up duplicating the
help text.  But again, it's not clear that any other arch wants to allow that,
i.e. we can cross that bridge if we come to it.

config ARCH_CONFIGURES_CPU_MITIGATIONS
	bool

if !ARCH_CONFIGURES_CPU_MITIGATIONS
config CPU_MITIGATIONS
	def_bool y
endif


> Regardless it seems very common for a menu "depends on" to be in a
> different file.  We could put CPU_MITIGATIONS in arch/Kconfig which is a
> fairly logical place for the dependency.

Yeah, arch/Kconfig is probably better than init/Kconfig.

Given that it's late on Friday, I'll somewhat speculatively (ba-dump ching!) post
a v2, and Cc Linus to explain the mess so that he can apply it directly if he
thinks it's urgent enough to squeeze into -rc5, and if if my idea isn't completely
off the rails.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-04-19 23:57 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-17  0:15 [PATCH 0/2] cpu: Fix default mitigation behavior Sean Christopherson
2024-04-17  0:15 ` [PATCH 1/2] cpu: Re-enable CPU mitigations by default for !X86 architectures Sean Christopherson
2024-04-18  0:54   ` Michael Ellerman
2024-04-19 14:27   ` Geert Uytterhoeven
2024-04-19 14:37   ` Will Deacon
2024-04-19 16:05   ` Josh Poimboeuf
2024-04-19 16:46     ` Sean Christopherson
2024-04-19 17:34       ` Josh Poimboeuf
2024-04-19 23:57         ` Sean Christopherson
2024-04-19 23:27     ` Michael Ellerman
2024-04-17  0:15 ` [PATCH 2/2] cpu: Ignore "mitigations" kernel parameter if CPU_MITIGATIONS=n Sean Christopherson
2024-04-19 15:00   ` Borislav Petkov
2024-04-19 16:01     ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).