* [PATCH v3] x86/bugs: Explicitly clear speculative MSR bits
@ 2022-11-28 15:31 Breno Leitao
2023-01-11 12:51 ` Borislav Petkov
2023-01-12 10:41 ` [tip: x86/cpu] x86/bugs: Reset speculation control settings on init tip-bot2 for Breno Leitao
0 siblings, 2 replies; 5+ messages in thread
From: Breno Leitao @ 2022-11-28 15:31 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, hpa, jpoimboe, peterz
Cc: x86, cascardo, leit, kexec, linux-kernel, Pawan Gupta
Currently x86_spec_ctrl_base is read at boot time, and speculative bits
are set if configs are enable, such as MSR[SPEC_CTRL_IBRS] is enabled if
CONFIG_CPU_IBRS_ENTRY is configured. These MSR bits are not cleared if
the mitigations are disabled.
This is a problem when kexec-ing a kernel that has the mitigation
disabled, from a kernel that has the mitigation enabled. In this case,
the MSR bits are carried forward and not cleared at the boot of the new
kernel. This might have some performance degradation that is hard to
find.
This problem does not happen if the machine is (hard) rebooted, because
the bit will be cleared by default.
Suggested-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
arch/x86/include/asm/msr-index.h | 4 ++++
arch/x86/kernel/cpu/bugs.c | 10 +++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 4a2af82553e4..22986a8f18bc 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -54,6 +54,10 @@
#define SPEC_CTRL_RRSBA_DIS_S_SHIFT 6 /* Disable RRSBA behavior */
#define SPEC_CTRL_RRSBA_DIS_S BIT(SPEC_CTRL_RRSBA_DIS_S_SHIFT)
+/* A mask for bits which the kernel toggles when controlling mitigations */
+#define SPEC_CTRL_MITIGATIONS_MASK (SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD \
+ | SPEC_CTRL_RRSBA_DIS_S)
+
#define MSR_IA32_PRED_CMD 0x00000049 /* Prediction Command */
#define PRED_CMD_IBPB BIT(0) /* Indirect Branch Prediction Barrier */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 3e3230cccaa7..4030358216c8 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -137,8 +137,16 @@ void __init check_bugs(void)
* have unknown values. AMD64_LS_CFG MSR is cached in the early AMD
* init code as it is not enumerated and depends on the family.
*/
- if (boot_cpu_has(X86_FEATURE_MSR_SPEC_CTRL))
+ if (boot_cpu_has(X86_FEATURE_MSR_SPEC_CTRL)) {
rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+ /*
+ * Previously running software, like kexec for example, may
+ * have some controls turned ON.
+ * Clear them and let the mitigations setup below set them
+ * based on configuration.
+ */
+ x86_spec_ctrl_base &= ~SPEC_CTRL_MITIGATIONS_MASK;
+ }
/* Select the proper CPU mitigations before patching alternatives: */
spectre_v1_select_mitigation();
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3] x86/bugs: Explicitly clear speculative MSR bits
2022-11-28 15:31 [PATCH v3] x86/bugs: Explicitly clear speculative MSR bits Breno Leitao
@ 2023-01-11 12:51 ` Borislav Petkov
2023-01-12 7:00 ` Pawan Gupta
2023-01-12 10:41 ` [tip: x86/cpu] x86/bugs: Reset speculation control settings on init tip-bot2 for Breno Leitao
1 sibling, 1 reply; 5+ messages in thread
From: Borislav Petkov @ 2023-01-11 12:51 UTC (permalink / raw)
To: Breno Leitao, Pawan Gupta
Cc: tglx, mingo, dave.hansen, hpa, jpoimboe, peterz, x86, cascardo,
leit, kexec, linux-kernel
On Mon, Nov 28, 2022 at 07:31:48AM -0800, Breno Leitao wrote:
> Currently x86_spec_ctrl_base is read at boot time, and speculative bits
> are set if configs are enable, such as MSR[SPEC_CTRL_IBRS] is enabled if
> CONFIG_CPU_IBRS_ENTRY is configured. These MSR bits are not cleared if
> the mitigations are disabled.
>
> This is a problem when kexec-ing a kernel that has the mitigation
> disabled, from a kernel that has the mitigation enabled. In this case,
> the MSR bits are carried forward and not cleared at the boot of the new
> kernel. This might have some performance degradation that is hard to
> find.
>
> This problem does not happen if the machine is (hard) rebooted, because
> the bit will be cleared by default.
>
> Suggested-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> arch/x86/include/asm/msr-index.h | 4 ++++
> arch/x86/kernel/cpu/bugs.c | 10 +++++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 4a2af82553e4..22986a8f18bc 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -54,6 +54,10 @@
> #define SPEC_CTRL_RRSBA_DIS_S_SHIFT 6 /* Disable RRSBA behavior */
> #define SPEC_CTRL_RRSBA_DIS_S BIT(SPEC_CTRL_RRSBA_DIS_S_SHIFT)
>
> +/* A mask for bits which the kernel toggles when controlling mitigations */
> +#define SPEC_CTRL_MITIGATIONS_MASK (SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD \
> + | SPEC_CTRL_RRSBA_DIS_S)
SPEC_CTRL_RRSBA_DIS_S is a disable bit and I presume it needs to stay enabled.
Only when spec_ctrl_disable_kernel_rrsba() runs. And I'd say perf-wise it
doesn't cost that much...
Pawan?
> +
> #define MSR_IA32_PRED_CMD 0x00000049 /* Prediction Command */
> #define PRED_CMD_IBPB BIT(0) /* Indirect Branch Prediction Barrier */
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 3e3230cccaa7..4030358216c8 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -137,8 +137,16 @@ void __init check_bugs(void)
> * have unknown values. AMD64_LS_CFG MSR is cached in the early AMD
> * init code as it is not enumerated and depends on the family.
> */
> - if (boot_cpu_has(X86_FEATURE_MSR_SPEC_CTRL))
> + if (boot_cpu_has(X86_FEATURE_MSR_SPEC_CTRL)) {
> rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
> + /*
> + * Previously running software, like kexec for example, may
> + * have some controls turned ON.
> + * Clear them and let the mitigations setup below set them
> + * based on configuration.
> + */
/*
* Previously running kernel (kexec), may have some controls
* turned ON. Clear them and let the mitigations setup below
* rediscover them based on configuration.
*/
There's no "previously running software, like kexec".
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] x86/bugs: Explicitly clear speculative MSR bits
2023-01-11 12:51 ` Borislav Petkov
@ 2023-01-12 7:00 ` Pawan Gupta
2023-01-12 10:34 ` Borislav Petkov
0 siblings, 1 reply; 5+ messages in thread
From: Pawan Gupta @ 2023-01-12 7:00 UTC (permalink / raw)
To: Borislav Petkov
Cc: Breno Leitao, tglx, mingo, dave.hansen, hpa, jpoimboe, peterz,
x86, cascardo, leit, kexec, linux-kernel
On Wed, Jan 11, 2023 at 01:51:03PM +0100, Borislav Petkov wrote:
> On Mon, Nov 28, 2022 at 07:31:48AM -0800, Breno Leitao wrote:
> > Currently x86_spec_ctrl_base is read at boot time, and speculative bits
> > are set if configs are enable, such as MSR[SPEC_CTRL_IBRS] is enabled if
> > CONFIG_CPU_IBRS_ENTRY is configured. These MSR bits are not cleared if
> > the mitigations are disabled.
> >
> > This is a problem when kexec-ing a kernel that has the mitigation
> > disabled, from a kernel that has the mitigation enabled. In this case,
> > the MSR bits are carried forward and not cleared at the boot of the new
> > kernel. This might have some performance degradation that is hard to
> > find.
> >
> > This problem does not happen if the machine is (hard) rebooted, because
> > the bit will be cleared by default.
> >
> > Suggested-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > ---
> > arch/x86/include/asm/msr-index.h | 4 ++++
> > arch/x86/kernel/cpu/bugs.c | 10 +++++++++-
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> > index 4a2af82553e4..22986a8f18bc 100644
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -54,6 +54,10 @@
> > #define SPEC_CTRL_RRSBA_DIS_S_SHIFT 6 /* Disable RRSBA behavior */
> > #define SPEC_CTRL_RRSBA_DIS_S BIT(SPEC_CTRL_RRSBA_DIS_S_SHIFT)
> >
> > +/* A mask for bits which the kernel toggles when controlling mitigations */
> > +#define SPEC_CTRL_MITIGATIONS_MASK (SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD \
> > + | SPEC_CTRL_RRSBA_DIS_S)
>
> SPEC_CTRL_RRSBA_DIS_S is a disable bit and I presume it needs to stay enabled.
The mitigation is enabled when this bit is set. When set, it prevents RET
target to be predicted from alternate predictors (BTB). This should stay
0, unless enabled by a mitigation mode.
> Only when spec_ctrl_disable_kernel_rrsba() runs. And I'd say perf-wise it
> doesn't cost that much...
I guess this doesn't matter now, because this patch is resetting it by
default that keeps the mitigation disabled with no perf impact.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] x86/bugs: Explicitly clear speculative MSR bits
2023-01-12 7:00 ` Pawan Gupta
@ 2023-01-12 10:34 ` Borislav Petkov
0 siblings, 0 replies; 5+ messages in thread
From: Borislav Petkov @ 2023-01-12 10:34 UTC (permalink / raw)
To: Pawan Gupta
Cc: Breno Leitao, tglx, mingo, dave.hansen, hpa, jpoimboe, peterz,
x86, cascardo, leit, kexec, linux-kernel
On Wed, Jan 11, 2023 at 11:00:37PM -0800, Pawan Gupta wrote:
> > SPEC_CTRL_RRSBA_DIS_S is a disable bit and I presume it needs to stay enabled.
>
> The mitigation is enabled when this bit is set. When set, it prevents RET
> target to be predicted from alternate predictors (BTB). This should stay
> 0, unless enabled by a mitigation mode.
>
> > Only when spec_ctrl_disable_kernel_rrsba() runs. And I'd say perf-wise it
> > doesn't cost that much...
>
> I guess this doesn't matter now, because this patch is resetting it by
> default that keeps the mitigation disabled with no perf impact.
Ok, lemme queue it then.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip: x86/cpu] x86/bugs: Reset speculation control settings on init
2022-11-28 15:31 [PATCH v3] x86/bugs: Explicitly clear speculative MSR bits Breno Leitao
2023-01-11 12:51 ` Borislav Petkov
@ 2023-01-12 10:41 ` tip-bot2 for Breno Leitao
1 sibling, 0 replies; 5+ messages in thread
From: tip-bot2 for Breno Leitao @ 2023-01-12 10:41 UTC (permalink / raw)
To: linux-tip-commits
Cc: Pawan Gupta, Breno Leitao, Borislav Petkov (AMD), x86, linux-kernel
The following commit has been merged into the x86/cpu branch of tip:
Commit-ID: 0125acda7d76b943ca55811df40ed6ec0ecf670f
Gitweb: https://git.kernel.org/tip/0125acda7d76b943ca55811df40ed6ec0ecf670f
Author: Breno Leitao <leitao@debian.org>
AuthorDate: Mon, 28 Nov 2022 07:31:48 -08:00
Committer: Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Thu, 12 Jan 2023 11:37:01 +01:00
x86/bugs: Reset speculation control settings on init
Currently, x86_spec_ctrl_base is read at boot time and speculative bits
are set if Kconfig items are enabled. For example, IBRS is enabled if
CONFIG_CPU_IBRS_ENTRY is configured, etc. These MSR bits are not cleared
if the mitigations are disabled.
This is a problem when kexec-ing a kernel that has the mitigation
disabled from a kernel that has the mitigation enabled. In this case,
the MSR bits are not cleared during the new kernel boot. As a result,
this might have some performance degradation that is hard to pinpoint.
This problem does not happen if the machine is (hard) rebooted because
the bit will be cleared by default.
[ bp: Massage. ]
Suggested-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/r/20221128153148.1129350-1-leitao@debian.org
---
arch/x86/include/asm/msr-index.h | 4 ++++
arch/x86/kernel/cpu/bugs.c | 10 +++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 37ff475..cb359d6 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -49,6 +49,10 @@
#define SPEC_CTRL_RRSBA_DIS_S_SHIFT 6 /* Disable RRSBA behavior */
#define SPEC_CTRL_RRSBA_DIS_S BIT(SPEC_CTRL_RRSBA_DIS_S_SHIFT)
+/* A mask for bits which the kernel toggles when controlling mitigations */
+#define SPEC_CTRL_MITIGATIONS_MASK (SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD \
+ | SPEC_CTRL_RRSBA_DIS_S)
+
#define MSR_IA32_PRED_CMD 0x00000049 /* Prediction Command */
#define PRED_CMD_IBPB BIT(0) /* Indirect Branch Prediction Barrier */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 19e1ce0..5f33704 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -145,9 +145,17 @@ void __init check_bugs(void)
* have unknown values. AMD64_LS_CFG MSR is cached in the early AMD
* init code as it is not enumerated and depends on the family.
*/
- if (boot_cpu_has(X86_FEATURE_MSR_SPEC_CTRL))
+ if (cpu_feature_enabled(X86_FEATURE_MSR_SPEC_CTRL)) {
rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+ /*
+ * Previously running kernel (kexec), may have some controls
+ * turned ON. Clear them and let the mitigations setup below
+ * rediscover them based on configuration.
+ */
+ x86_spec_ctrl_base &= ~SPEC_CTRL_MITIGATIONS_MASK;
+ }
+
/* Select the proper CPU mitigations before patching alternatives: */
spectre_v1_select_mitigation();
spectre_v2_select_mitigation();
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-01-12 10:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-28 15:31 [PATCH v3] x86/bugs: Explicitly clear speculative MSR bits Breno Leitao
2023-01-11 12:51 ` Borislav Petkov
2023-01-12 7:00 ` Pawan Gupta
2023-01-12 10:34 ` Borislav Petkov
2023-01-12 10:41 ` [tip: x86/cpu] x86/bugs: Reset speculation control settings on init tip-bot2 for Breno Leitao
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 NNTP newsgroup(s).