All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts
@ 2022-07-14  5:32 Pawan Gupta
  2022-07-14 11:17 ` Thadeu Lima de Souza Cascardo
  2022-07-14 23:15 ` [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is " Pawan Gupta
  0 siblings, 2 replies; 14+ messages in thread
From: Pawan Gupta @ 2022-07-14  5:32 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-kernel, stable, Daniel Sneddon, antonio.gomez.iglesias,
	Josh Poimboeuf

Currently spectre_v2=ibrs forces write to MSR_IA32_SPEC_CTRL at every
entry and exit. On Enhanced IBRS parts setting MSR_IA32_SPEC_CTRL[IBRS]
only once at bootup is sufficient. MSR write at every kernel entry/exit
incur unnecessary penalty that can be avoided.

When Enhanced IBRS feature is present, switch from "ibrs" to "auto" mode
so that appropriate mitigation is selected.

Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/kernel/cpu/bugs.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 0dd04713434b..7d7ebfdfbeda 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1303,6 +1303,12 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
 		return SPECTRE_V2_CMD_AUTO;
 	}
 
+	if (cmd == SPECTRE_V2_CMD_IBRS && boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+		pr_err("%s selected but CPU supports Enhanced IBRS. Switching to AUTO select\n",
+		       mitigation_options[i].option);
+		return SPECTRE_V2_CMD_AUTO;
+	}
+
 	spec_v2_print_cond(mitigation_options[i].option,
 			   mitigation_options[i].secure);
 	return cmd;

base-commit: 72a8e05d4f66b5af7854df4490e3135168694b6b
-- 
2.35.3



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

* Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts
  2022-07-14  5:32 [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts Pawan Gupta
@ 2022-07-14 11:17 ` Thadeu Lima de Souza Cascardo
  2022-07-14 11:42   ` Peter Zijlstra
  2022-07-14 23:15 ` [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is " Pawan Gupta
  1 sibling, 1 reply; 14+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2022-07-14 11:17 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, linux-kernel, stable, Daniel Sneddon,
	antonio.gomez.iglesias, Josh Poimboeuf

On Wed, Jul 13, 2022 at 10:32:37PM -0700, Pawan Gupta wrote:
> Currently spectre_v2=ibrs forces write to MSR_IA32_SPEC_CTRL at every
> entry and exit. On Enhanced IBRS parts setting MSR_IA32_SPEC_CTRL[IBRS]
> only once at bootup is sufficient. MSR write at every kernel entry/exit
> incur unnecessary penalty that can be avoided.
> 
> When Enhanced IBRS feature is present, switch from "ibrs" to "auto" mode
> so that appropriate mitigation is selected.
> 
> Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
> Cc: stable@vger.kernel.org # 5.10+
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/bugs.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 0dd04713434b..7d7ebfdfbeda 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -1303,6 +1303,12 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
>  		return SPECTRE_V2_CMD_AUTO;
>  	}
>  
> +	if (cmd == SPECTRE_V2_CMD_IBRS && boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> +		pr_err("%s selected but CPU supports Enhanced IBRS. Switching to AUTO select\n",
> +		       mitigation_options[i].option);
> +		return SPECTRE_V2_CMD_AUTO;
> +	}
> +
>  	spec_v2_print_cond(mitigation_options[i].option,
>  			   mitigation_options[i].secure);
>  	return cmd;
> 
> base-commit: 72a8e05d4f66b5af7854df4490e3135168694b6b
> -- 
> 2.35.3
> 
> 

Shouldn't we just use the mitigation the user asked for if it is still
possible? We could add the warning advising the user that a different
mitigation could be used instead with less penalty, but if the user asked for
IBRS and that is available, it should be used.

One of the reasons for that is testing. I know it was useful enough for me and
it helped me find some bugs.

Cascardo.

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

* Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts
  2022-07-14 11:17 ` Thadeu Lima de Souza Cascardo
@ 2022-07-14 11:42   ` Peter Zijlstra
  2022-07-14 14:59     ` Pawan Gupta
  2022-07-14 16:01     ` Josh Poimboeuf
  0 siblings, 2 replies; 14+ messages in thread
From: Peter Zijlstra @ 2022-07-14 11:42 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo
  Cc: Pawan Gupta, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, linux-kernel, stable,
	Daniel Sneddon, antonio.gomez.iglesias, Josh Poimboeuf

On Thu, Jul 14, 2022 at 08:17:26AM -0300, Thadeu Lima de Souza Cascardo wrote:
> On Wed, Jul 13, 2022 at 10:32:37PM -0700, Pawan Gupta wrote:
> > Currently spectre_v2=ibrs forces write to MSR_IA32_SPEC_CTRL at every
> > entry and exit. On Enhanced IBRS parts setting MSR_IA32_SPEC_CTRL[IBRS]
> > only once at bootup is sufficient. MSR write at every kernel entry/exit
> > incur unnecessary penalty that can be avoided.
> > 
> > When Enhanced IBRS feature is present, switch from "ibrs" to "auto" mode
> > so that appropriate mitigation is selected.
> > 
> > Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
> > Cc: stable@vger.kernel.org # 5.10+
> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > ---
> >  arch/x86/kernel/cpu/bugs.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > index 0dd04713434b..7d7ebfdfbeda 100644
> > --- a/arch/x86/kernel/cpu/bugs.c
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -1303,6 +1303,12 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
> >  		return SPECTRE_V2_CMD_AUTO;
> >  	}
> >  
> > +	if (cmd == SPECTRE_V2_CMD_IBRS && boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> > +		pr_err("%s selected but CPU supports Enhanced IBRS. Switching to AUTO select\n",
> > +		       mitigation_options[i].option);
> > +		return SPECTRE_V2_CMD_AUTO;
> > +	}
> > +
> >  	spec_v2_print_cond(mitigation_options[i].option,
> >  			   mitigation_options[i].secure);
> >  	return cmd;
> > 
> > base-commit: 72a8e05d4f66b5af7854df4490e3135168694b6b
> > -- 
> > 2.35.3
> > 
> > 
> 
> Shouldn't we just use the mitigation the user asked for if it is still
> possible? We could add the warning advising the user that a different
> mitigation could be used instead with less penalty, but if the user asked for
> IBRS and that is available, it should be used.
> 
> One of the reasons for that is testing. I know it was useful enough for me and
> it helped me find some bugs.

Yeah this; if the user asks for IBRS, we should give him IBRS. I hate
the 'I know better, let me change that for you' mentality.

If you want to do something, print a warning.

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

* Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts
  2022-07-14 11:42   ` Peter Zijlstra
@ 2022-07-14 14:59     ` Pawan Gupta
  2022-07-14 16:01     ` Josh Poimboeuf
  1 sibling, 0 replies; 14+ messages in thread
From: Pawan Gupta @ 2022-07-14 14:59 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Thadeu Lima de Souza Cascardo, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, linux-kernel,
	stable, Daniel Sneddon, antonio.gomez.iglesias, Josh Poimboeuf

On Thu, Jul 14, 2022 at 01:42:11PM +0200, Peter Zijlstra wrote:
>On Thu, Jul 14, 2022 at 08:17:26AM -0300, Thadeu Lima de Souza Cascardo wrote:
>> On Wed, Jul 13, 2022 at 10:32:37PM -0700, Pawan Gupta wrote:
>> > Currently spectre_v2=ibrs forces write to MSR_IA32_SPEC_CTRL at every
>> > entry and exit. On Enhanced IBRS parts setting MSR_IA32_SPEC_CTRL[IBRS]
>> > only once at bootup is sufficient. MSR write at every kernel entry/exit
>> > incur unnecessary penalty that can be avoided.
>> >
>> > When Enhanced IBRS feature is present, switch from "ibrs" to "auto" mode
>> > so that appropriate mitigation is selected.
>> >
>> > Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
>> > Cc: stable@vger.kernel.org # 5.10+
>> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
>> > ---
>> >  arch/x86/kernel/cpu/bugs.c | 6 ++++++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
>> > index 0dd04713434b..7d7ebfdfbeda 100644
>> > --- a/arch/x86/kernel/cpu/bugs.c
>> > +++ b/arch/x86/kernel/cpu/bugs.c
>> > @@ -1303,6 +1303,12 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
>> >  		return SPECTRE_V2_CMD_AUTO;
>> >  	}
>> >
>> > +	if (cmd == SPECTRE_V2_CMD_IBRS && boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
>> > +		pr_err("%s selected but CPU supports Enhanced IBRS. Switching to AUTO select\n",
>> > +		       mitigation_options[i].option);
>> > +		return SPECTRE_V2_CMD_AUTO;
>> > +	}
>> > +
>> >  	spec_v2_print_cond(mitigation_options[i].option,
>> >  			   mitigation_options[i].secure);
>> >  	return cmd;
>> >
>> > base-commit: 72a8e05d4f66b5af7854df4490e3135168694b6b
>> > --
>> > 2.35.3
>> >
>> >
>>
>> Shouldn't we just use the mitigation the user asked for if it is still
>> possible? We could add the warning advising the user that a different
>> mitigation could be used instead with less penalty, but if the user asked for
>> IBRS and that is available, it should be used.
>>
>> One of the reasons for that is testing. I know it was useful enough for me and
>> it helped me find some bugs.
>
>Yeah this; if the user asks for IBRS, we should give him IBRS. I hate
>the 'I know better, let me change that for you' mentality.
>
>If you want to do something, print a warning.

Fair enough, I will change that to a warning.

Thanks,
Pawan

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

* Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts
  2022-07-14 11:42   ` Peter Zijlstra
  2022-07-14 14:59     ` Pawan Gupta
@ 2022-07-14 16:01     ` Josh Poimboeuf
  2022-07-14 17:03       ` Peter Zijlstra
  1 sibling, 1 reply; 14+ messages in thread
From: Josh Poimboeuf @ 2022-07-14 16:01 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Thadeu Lima de Souza Cascardo, Pawan Gupta, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	linux-kernel, stable, Daniel Sneddon, antonio.gomez.iglesias

On Thu, Jul 14, 2022 at 01:42:11PM +0200, Peter Zijlstra wrote:
> On Thu, Jul 14, 2022 at 08:17:26AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > On Wed, Jul 13, 2022 at 10:32:37PM -0700, Pawan Gupta wrote:
> > > Currently spectre_v2=ibrs forces write to MSR_IA32_SPEC_CTRL at every
> > > entry and exit. On Enhanced IBRS parts setting MSR_IA32_SPEC_CTRL[IBRS]
> > > only once at bootup is sufficient. MSR write at every kernel entry/exit
> > > incur unnecessary penalty that can be avoided.
> > > 
> > > When Enhanced IBRS feature is present, switch from "ibrs" to "auto" mode
> > > so that appropriate mitigation is selected.
> > > 
> > > Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
> > > Cc: stable@vger.kernel.org # 5.10+
> > > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > > ---
> > >  arch/x86/kernel/cpu/bugs.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > > index 0dd04713434b..7d7ebfdfbeda 100644
> > > --- a/arch/x86/kernel/cpu/bugs.c
> > > +++ b/arch/x86/kernel/cpu/bugs.c
> > > @@ -1303,6 +1303,12 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
> > >  		return SPECTRE_V2_CMD_AUTO;
> > >  	}
> > >  
> > > +	if (cmd == SPECTRE_V2_CMD_IBRS && boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> > > +		pr_err("%s selected but CPU supports Enhanced IBRS. Switching to AUTO select\n",
> > > +		       mitigation_options[i].option);
> > > +		return SPECTRE_V2_CMD_AUTO;
> > > +	}
> > > +
> > >  	spec_v2_print_cond(mitigation_options[i].option,
> > >  			   mitigation_options[i].secure);
> > >  	return cmd;
> > > 
> > > base-commit: 72a8e05d4f66b5af7854df4490e3135168694b6b
> > > -- 
> > > 2.35.3
> > > 
> > > 
> > 
> > Shouldn't we just use the mitigation the user asked for if it is still
> > possible? We could add the warning advising the user that a different
> > mitigation could be used instead with less penalty, but if the user asked for
> > IBRS and that is available, it should be used.
> > 
> > One of the reasons for that is testing. I know it was useful enough for me and
> > it helped me find some bugs.
> 
> Yeah this; if the user asks for IBRS, we should give him IBRS. I hate
> the 'I know better, let me change that for you' mentality.

eIBRS CPUs don't even have legacy IBRS so I don't see how this is even
possible.

-- 
Josh

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

* Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts
  2022-07-14 16:01     ` Josh Poimboeuf
@ 2022-07-14 17:03       ` Peter Zijlstra
  2022-07-14 17:38         ` Josh Poimboeuf
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2022-07-14 17:03 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Thadeu Lima de Souza Cascardo, Pawan Gupta, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	linux-kernel, stable, Daniel Sneddon, antonio.gomez.iglesias

On Thu, Jul 14, 2022 at 09:01:06AM -0700, Josh Poimboeuf wrote:

> > Yeah this; if the user asks for IBRS, we should give him IBRS. I hate
> > the 'I know better, let me change that for you' mentality.
> 
> eIBRS CPUs don't even have legacy IBRS so I don't see how this is even
> possible.

You can still WRMSR a lot on them. Might not make sense but it 'works'.

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

* Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts
  2022-07-14 17:03       ` Peter Zijlstra
@ 2022-07-14 17:38         ` Josh Poimboeuf
  2022-07-14 18:11           ` Peter Zijlstra
  2022-07-14 18:42           ` Pawan Gupta
  0 siblings, 2 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2022-07-14 17:38 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Thadeu Lima de Souza Cascardo, Pawan Gupta, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	linux-kernel, stable, Daniel Sneddon, antonio.gomez.iglesias

On Thu, Jul 14, 2022 at 07:03:32PM +0200, Peter Zijlstra wrote:
> On Thu, Jul 14, 2022 at 09:01:06AM -0700, Josh Poimboeuf wrote:
> 
> > > Yeah this; if the user asks for IBRS, we should give him IBRS. I hate
> > > the 'I know better, let me change that for you' mentality.
> > 
> > eIBRS CPUs don't even have legacy IBRS so I don't see how this is even
> > possible.
> 
> You can still WRMSR a lot on them. Might not make sense but it 'works'.

Even in Intel documentation, eIBRS is often referred to as IBRS. It
wouldn't be surprising for a user to consider spectre_v2=ibrs to mean
"use eIBRS".

I'm pretty sure there's nobody out there that wants spectre_v2=ibrs to
mean "make it slower and possibly less secure because it's being used
contrary to the spec".

-- 
Josh

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

* Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts
  2022-07-14 17:38         ` Josh Poimboeuf
@ 2022-07-14 18:11           ` Peter Zijlstra
  2022-07-14 18:42           ` Pawan Gupta
  1 sibling, 0 replies; 14+ messages in thread
From: Peter Zijlstra @ 2022-07-14 18:11 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Thadeu Lima de Souza Cascardo, Pawan Gupta, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	linux-kernel, stable, Daniel Sneddon, antonio.gomez.iglesias

On Thu, Jul 14, 2022 at 10:38:14AM -0700, Josh Poimboeuf wrote:
> On Thu, Jul 14, 2022 at 07:03:32PM +0200, Peter Zijlstra wrote:
> > On Thu, Jul 14, 2022 at 09:01:06AM -0700, Josh Poimboeuf wrote:
> > 
> > > > Yeah this; if the user asks for IBRS, we should give him IBRS. I hate
> > > > the 'I know better, let me change that for you' mentality.
> > > 
> > > eIBRS CPUs don't even have legacy IBRS so I don't see how this is even
> > > possible.
> > 
> > You can still WRMSR a lot on them. Might not make sense but it 'works'.
> 
> Even in Intel documentation, eIBRS is often referred to as IBRS. It
> wouldn't be surprising for a user to consider spectre_v2=ibrs to mean
> "use eIBRS".
> 
> I'm pretty sure there's nobody out there that wants spectre_v2=ibrs to
> mean "make it slower and possibly less secure because it's being used
> contrary to the spec".

Then make it print a big honking warning.

Most people will either use auto or off, the very few people that force
an option get what they ask for, not something else.

Like said upthread, it allows testing the code-paths at the very least.

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

* Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts
  2022-07-14 17:38         ` Josh Poimboeuf
  2022-07-14 18:11           ` Peter Zijlstra
@ 2022-07-14 18:42           ` Pawan Gupta
  1 sibling, 0 replies; 14+ messages in thread
From: Pawan Gupta @ 2022-07-14 18:42 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Peter Zijlstra, Thadeu Lima de Souza Cascardo, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	linux-kernel, stable, Daniel Sneddon, antonio.gomez.iglesias

On Thu, Jul 14, 2022 at 10:38:14AM -0700, Josh Poimboeuf wrote:
>On Thu, Jul 14, 2022 at 07:03:32PM +0200, Peter Zijlstra wrote:
>> On Thu, Jul 14, 2022 at 09:01:06AM -0700, Josh Poimboeuf wrote:
>>
>> > > Yeah this; if the user asks for IBRS, we should give him IBRS. I hate
>> > > the 'I know better, let me change that for you' mentality.
>> >
>> > eIBRS CPUs don't even have legacy IBRS so I don't see how this is even
>> > possible.
>>
>> You can still WRMSR a lot on them. Might not make sense but it 'works'.
>
>Even in Intel documentation, eIBRS is often referred to as IBRS. It
>wouldn't be surprising for a user to consider spectre_v2=ibrs to mean
>"use eIBRS".
>
>I'm pretty sure there's nobody out there that wants spectre_v2=ibrs to
>mean "make it slower and possibly less secure because it's being used
>contrary to the spec".

Apart from testing, I don't see a reason for a user to deliberately
choose =ibrs on Enhanced IBRS parts. But, I am guessing most users would
just rely on "=auto" mode.

So honoring what the user asked and printing a warning may be fine. And
hope they would see the warning if they unintentionally chose "=ibrs" on
an eIBRS part.

Thanks,
Pawan

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

* [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is selected on Enhanced IBRS parts
  2022-07-14  5:32 [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts Pawan Gupta
  2022-07-14 11:17 ` Thadeu Lima de Souza Cascardo
@ 2022-07-14 23:15 ` Pawan Gupta
  2022-07-15  0:11   ` Thadeu Lima de Souza Cascardo
  2022-07-15  5:25   ` Greg KH
  1 sibling, 2 replies; 14+ messages in thread
From: Pawan Gupta @ 2022-07-14 23:15 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-kernel, stable, Daniel Sneddon, antonio.gomez.iglesias,
	Josh Poimboeuf

IBRS mitigation for spectre_v2 forces write to MSR_IA32_SPEC_CTRL at
every kernel entry/exit. On Enhanced IBRS parts setting
MSR_IA32_SPEC_CTRL[IBRS] only once at boot is sufficient. MSR writes at
every kernel entry/exit incur unnecessary performance loss.

When Enhanced IBRS feature is present, print a warning about this
unnecessary performance loss.

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
v1->v2: Instead of changing the mitigation, print a warning about the
        perf loss.

v1: https://lore.kernel.org/lkml/0456b35fb9ef957d9a9138e0913fb1a3fd445dff.1657747493.git.pawan.kumar.gupta@linux.intel.com/

 arch/x86/kernel/cpu/bugs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 0dd04713434b..1c54fad3c54b 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -975,6 +975,7 @@ static inline const char *spectre_v2_module_string(void) { return ""; }
 #define SPECTRE_V2_LFENCE_MSG "WARNING: LFENCE mitigation is not recommended for this CPU, data leaks possible!\n"
 #define SPECTRE_V2_EIBRS_EBPF_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS on, data leaks possible via Spectre v2 BHB attacks!\n"
 #define SPECTRE_V2_EIBRS_LFENCE_EBPF_SMT_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS+LFENCE mitigation and SMT, data leaks possible via Spectre v2 BHB attacks!\n"
+#define SPECTRE_V2_IBRS_PERF_MSG "WARNING: IBRS mitigation selected on Enhanced IBRS CPU, this may cause unnecessary performance loss\n"
 
 #ifdef CONFIG_BPF_SYSCALL
 void unpriv_ebpf_notify(int new_state)
@@ -1415,6 +1416,8 @@ static void __init spectre_v2_select_mitigation(void)
 
 	case SPECTRE_V2_IBRS:
 		setup_force_cpu_cap(X86_FEATURE_KERNEL_IBRS);
+		if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED))
+			pr_warn(SPECTRE_V2_IBRS_PERF_MSG);
 		break;
 
 	case SPECTRE_V2_LFENCE:

base-commit: 4a57a8400075bc5287c5c877702c68aeae2a033d
-- 
2.35.3



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

* Re: [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is selected on Enhanced IBRS parts
  2022-07-14 23:15 ` [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is " Pawan Gupta
@ 2022-07-15  0:11   ` Thadeu Lima de Souza Cascardo
  2022-07-15  2:00     ` Pawan Gupta
  2022-07-15  5:25   ` Greg KH
  1 sibling, 1 reply; 14+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2022-07-15  0:11 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, linux-kernel, stable, Daniel Sneddon,
	antonio.gomez.iglesias, Josh Poimboeuf

On Thu, Jul 14, 2022 at 04:15:35PM -0700, Pawan Gupta wrote:
> IBRS mitigation for spectre_v2 forces write to MSR_IA32_SPEC_CTRL at
> every kernel entry/exit. On Enhanced IBRS parts setting
> MSR_IA32_SPEC_CTRL[IBRS] only once at boot is sufficient. MSR writes at
> every kernel entry/exit incur unnecessary performance loss.
> 
> When Enhanced IBRS feature is present, print a warning about this
> unnecessary performance loss.
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>

Reviewed-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>

> ---
> v1->v2: Instead of changing the mitigation, print a warning about the
>         perf loss.
> 
> v1: https://lore.kernel.org/lkml/0456b35fb9ef957d9a9138e0913fb1a3fd445dff.1657747493.git.pawan.kumar.gupta@linux.intel.com/
> 
>  arch/x86/kernel/cpu/bugs.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 0dd04713434b..1c54fad3c54b 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -975,6 +975,7 @@ static inline const char *spectre_v2_module_string(void) { return ""; }
>  #define SPECTRE_V2_LFENCE_MSG "WARNING: LFENCE mitigation is not recommended for this CPU, data leaks possible!\n"
>  #define SPECTRE_V2_EIBRS_EBPF_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS on, data leaks possible via Spectre v2 BHB attacks!\n"
>  #define SPECTRE_V2_EIBRS_LFENCE_EBPF_SMT_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS+LFENCE mitigation and SMT, data leaks possible via Spectre v2 BHB attacks!\n"
> +#define SPECTRE_V2_IBRS_PERF_MSG "WARNING: IBRS mitigation selected on Enhanced IBRS CPU, this may cause unnecessary performance loss\n"
>  
>  #ifdef CONFIG_BPF_SYSCALL
>  void unpriv_ebpf_notify(int new_state)
> @@ -1415,6 +1416,8 @@ static void __init spectre_v2_select_mitigation(void)
>  
>  	case SPECTRE_V2_IBRS:
>  		setup_force_cpu_cap(X86_FEATURE_KERNEL_IBRS);
> +		if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED))
> +			pr_warn(SPECTRE_V2_IBRS_PERF_MSG);
>  		break;
>  
>  	case SPECTRE_V2_LFENCE:
> 
> base-commit: 4a57a8400075bc5287c5c877702c68aeae2a033d
> -- 
> 2.35.3
> 
> 

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

* Re: [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is selected on Enhanced IBRS parts
  2022-07-15  0:11   ` Thadeu Lima de Souza Cascardo
@ 2022-07-15  2:00     ` Pawan Gupta
  0 siblings, 0 replies; 14+ messages in thread
From: Pawan Gupta @ 2022-07-15  2:00 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, linux-kernel, stable, Daniel Sneddon,
	antonio.gomez.iglesias, Josh Poimboeuf

On Thu, Jul 14, 2022 at 09:11:18PM -0300, Thadeu Lima de Souza Cascardo wrote:
>On Thu, Jul 14, 2022 at 04:15:35PM -0700, Pawan Gupta wrote:
>> IBRS mitigation for spectre_v2 forces write to MSR_IA32_SPEC_CTRL at
>> every kernel entry/exit. On Enhanced IBRS parts setting
>> MSR_IA32_SPEC_CTRL[IBRS] only once at boot is sufficient. MSR writes at
>> every kernel entry/exit incur unnecessary performance loss.
>>
>> When Enhanced IBRS feature is present, print a warning about this
>> unnecessary performance loss.
>>
>> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
>
>Reviewed-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>

Thanks for the review.

Pawan

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

* Re: [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is selected on Enhanced IBRS parts
  2022-07-14 23:15 ` [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is " Pawan Gupta
  2022-07-15  0:11   ` Thadeu Lima de Souza Cascardo
@ 2022-07-15  5:25   ` Greg KH
  2022-07-15 17:21     ` Pawan Gupta
  1 sibling, 1 reply; 14+ messages in thread
From: Greg KH @ 2022-07-15  5:25 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, linux-kernel, stable, Daniel Sneddon,
	antonio.gomez.iglesias, Josh Poimboeuf

On Thu, Jul 14, 2022 at 04:15:35PM -0700, Pawan Gupta wrote:
> IBRS mitigation for spectre_v2 forces write to MSR_IA32_SPEC_CTRL at
> every kernel entry/exit. On Enhanced IBRS parts setting
> MSR_IA32_SPEC_CTRL[IBRS] only once at boot is sufficient. MSR writes at
> every kernel entry/exit incur unnecessary performance loss.
> 
> When Enhanced IBRS feature is present, print a warning about this
> unnecessary performance loss.
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---
> v1->v2: Instead of changing the mitigation, print a warning about the
>         perf loss.
> 
> v1: https://lore.kernel.org/lkml/0456b35fb9ef957d9a9138e0913fb1a3fd445dff.1657747493.git.pawan.kumar.gupta@linux.intel.com/
> 
>  arch/x86/kernel/cpu/bugs.c | 3 +++
>  1 file changed, 3 insertions(+)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

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

* Re: [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is selected on Enhanced IBRS parts
  2022-07-15  5:25   ` Greg KH
@ 2022-07-15 17:21     ` Pawan Gupta
  0 siblings, 0 replies; 14+ messages in thread
From: Pawan Gupta @ 2022-07-15 17:21 UTC (permalink / raw)
  To: Greg KH
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, linux-kernel, Daniel Sneddon,
	antonio.gomez.iglesias, Josh Poimboeuf

On Fri, Jul 15, 2022 at 07:25:43AM +0200, Greg KH wrote:
> On Thu, Jul 14, 2022 at 04:15:35PM -0700, Pawan Gupta wrote:
> > IBRS mitigation for spectre_v2 forces write to MSR_IA32_SPEC_CTRL at
> > every kernel entry/exit. On Enhanced IBRS parts setting
> > MSR_IA32_SPEC_CTRL[IBRS] only once at boot is sufficient. MSR writes at
> > every kernel entry/exit incur unnecessary performance loss.
> > 
> > When Enhanced IBRS feature is present, print a warning about this
> > unnecessary performance loss.
> > 
> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > ---
> > v1->v2: Instead of changing the mitigation, print a warning about the
> >         perf loss.
> > 
> > v1: https://lore.kernel.org/lkml/0456b35fb9ef957d9a9138e0913fb1a3fd445dff.1657747493.git.pawan.kumar.gupta@linux.intel.com/
> > 
> >  arch/x86/kernel/cpu/bugs.c | 3 +++
> >  1 file changed, 3 insertions(+)
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>

Sorry, I CCed stable@ by mistake. This version just adds a warning, it
is not intended to be backported.

Thanks,
Pawan

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

end of thread, other threads:[~2022-07-15 17:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-14  5:32 [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts Pawan Gupta
2022-07-14 11:17 ` Thadeu Lima de Souza Cascardo
2022-07-14 11:42   ` Peter Zijlstra
2022-07-14 14:59     ` Pawan Gupta
2022-07-14 16:01     ` Josh Poimboeuf
2022-07-14 17:03       ` Peter Zijlstra
2022-07-14 17:38         ` Josh Poimboeuf
2022-07-14 18:11           ` Peter Zijlstra
2022-07-14 18:42           ` Pawan Gupta
2022-07-14 23:15 ` [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is " Pawan Gupta
2022-07-15  0:11   ` Thadeu Lima de Souza Cascardo
2022-07-15  2:00     ` Pawan Gupta
2022-07-15  5:25   ` Greg KH
2022-07-15 17:21     ` Pawan Gupta

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.