2022-07-14 06:00:54

by Pawan Gupta

[permalink] [raw]
Subject: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts

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: [email protected] # 5.10+
Signed-off-by: Pawan Gupta <[email protected]>
---
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



Subject: Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts

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: [email protected] # 5.10+
> Signed-off-by: Pawan Gupta <[email protected]>
> ---
> 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.

2022-07-14 11:56:58

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts

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: [email protected] # 5.10+
> > Signed-off-by: Pawan Gupta <[email protected]>
> > ---
> > 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.

2022-07-14 15:17:17

by Pawan Gupta

[permalink] [raw]
Subject: Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts

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: [email protected] # 5.10+
>> > Signed-off-by: Pawan Gupta <[email protected]>
>> > ---
>> > 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

2022-07-14 16:16:08

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts

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: [email protected] # 5.10+
> > > Signed-off-by: Pawan Gupta <[email protected]>
> > > ---
> > > 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

2022-07-14 17:07:17

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts

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'.

2022-07-14 18:25:49

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts

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

2022-07-14 18:46:01

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts

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.

2022-07-14 19:10:42

by Pawan Gupta

[permalink] [raw]
Subject: Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts

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

2022-07-14 23:17:13

by Pawan Gupta

[permalink] [raw]
Subject: [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is selected on Enhanced IBRS parts

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 <[email protected]>
---
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


Subject: Re: [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is selected on Enhanced IBRS parts

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 <[email protected]>

Reviewed-by: Thadeu Lima de Souza Cascardo <[email protected]>

> ---
> 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
>
>

2022-07-15 02:46:56

by Pawan Gupta

[permalink] [raw]
Subject: Re: [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is selected on Enhanced IBRS parts

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 <[email protected]>
>
>Reviewed-by: Thadeu Lima de Souza Cascardo <[email protected]>

Thanks for the review.

Pawan

2022-07-15 05:32:49

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is selected on Enhanced IBRS parts

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 <[email protected]>
> ---
> 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>

2022-07-15 17:30:47

by Pawan Gupta

[permalink] [raw]
Subject: Re: [PATCH v2] x86/bugs: Warn when "ibrs" mitigation is selected on Enhanced IBRS parts

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 <[email protected]>
> > ---
> > 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