For consistency with the other CONFIG_MITIGATION_* options, replace the
CONFIG_SPECTRE_BHI_{ON,OFF} options with a single
CONFIG_MITIGATION_SPECTRE_BHI option.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
arch/x86/Kconfig | 17 +++--------------
arch/x86/kernel/cpu/bugs.c | 2 +-
2 files changed, 4 insertions(+), 15 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b63b6767a63d..4474bf32d0a4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2633,27 +2633,16 @@ config MITIGATION_RFDS
stored in floating point, vector and integer registers.
See also <file:Documentation/admin-guide/hw-vuln/reg-file-data-sampling.rst>
-choice
- prompt "Clear branch history"
+config MITIGATION_SPECTRE_BHI
+ bool "Mitigate Spectre-BHB (Branch History Injection)"
depends on CPU_SUP_INTEL
- default SPECTRE_BHI_ON
+ default y
help
Enable BHI mitigations. BHI attacks are a form of Spectre V2 attacks
where the branch history buffer is poisoned to speculatively steer
indirect branches.
See <file:Documentation/admin-guide/hw-vuln/spectre.rst>
-config SPECTRE_BHI_ON
- bool "on"
- help
- Equivalent to setting spectre_bhi=on command line parameter.
-config SPECTRE_BHI_OFF
- bool "off"
- help
- Equivalent to setting spectre_bhi=off command line parameter.
-
-endchoice
-
endif
config ARCH_HAS_ADD_PAGES
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 74ade6d7caa3..4c46fa2d08c2 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1628,7 +1628,7 @@ enum bhi_mitigations {
};
static enum bhi_mitigations bhi_mitigation __ro_after_init =
- IS_ENABLED(CONFIG_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
+ IS_ENABLED(CONFIG_MITIGATION_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
static int __init spectre_bhi_parse_cmdline(char *str)
{
--
2.44.0
* Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> For consistency with the other CONFIG_MITIGATION_* options, replace the
> CONFIG_SPECTRE_BHI_{ON,OFF} options with a single
> CONFIG_MITIGATION_SPECTRE_BHI option.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
> arch/x86/Kconfig | 17 +++--------------
> arch/x86/kernel/cpu/bugs.c | 2 +-
> 2 files changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index b63b6767a63d..4474bf32d0a4 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2633,27 +2633,16 @@ config MITIGATION_RFDS
> stored in floating point, vector and integer registers.
> See also <file:Documentation/admin-guide/hw-vuln/reg-file-data-sampling.rst>
>
> -choice
> - prompt "Clear branch history"
> +config MITIGATION_SPECTRE_BHI
> + bool "Mitigate Spectre-BHB (Branch History Injection)"
> depends on CPU_SUP_INTEL
> - default SPECTRE_BHI_ON
> + default y
> help
> Enable BHI mitigations. BHI attacks are a form of Spectre V2 attacks
> where the branch history buffer is poisoned to speculatively steer
> indirect branches.
> See <file:Documentation/admin-guide/hw-vuln/spectre.rst>
>
> -config SPECTRE_BHI_ON
> - bool "on"
> - help
> - Equivalent to setting spectre_bhi=on command line parameter.
> -config SPECTRE_BHI_OFF
> - bool "off"
> - help
> - Equivalent to setting spectre_bhi=off command line parameter.
> -
> -endchoice
> -
> endif
>
> config ARCH_HAS_ADD_PAGES
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 74ade6d7caa3..4c46fa2d08c2 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -1628,7 +1628,7 @@ enum bhi_mitigations {
> };
>
> static enum bhi_mitigations bhi_mitigation __ro_after_init =
> - IS_ENABLED(CONFIG_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
> + IS_ENABLED(CONFIG_MITIGATION_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
Uhm, after this patch there's no CONFIG_MITIGATION_SPECTRE_BHI_ON anymore,
which is kindof nasty, as IS_ENABLED() doesn't generate a build failure for
non-existent Kconfig variables IIRC ...
So AFAICT this patch turns on BHI unconditionally.
I've fixed as per the patch below, but please double check the end result
in tip:x86/urgent once I've pushed it out..
Thanks,
Ingo
=====================>
arch/x86/kernel/cpu/bugs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 80d9018da3d2..25111ad388d9 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1628,7 +1628,7 @@ enum bhi_mitigations {
};
static enum bhi_mitigations bhi_mitigation __ro_after_init =
- IS_ENABLED(CONFIG_MITIGATION_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
+ IS_ENABLED(CONFIG_MITIGATION_SPECTRE_BHI) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
static int __init spectre_bhi_parse_cmdline(char *str)
{
On Thu, Apr 11, 2024 at 09:48:42AM +0200, Ingo Molnar wrote:
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -1628,7 +1628,7 @@ enum bhi_mitigations {
> > };
> >
> > static enum bhi_mitigations bhi_mitigation __ro_after_init =
> > - IS_ENABLED(CONFIG_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
> > + IS_ENABLED(CONFIG_MITIGATION_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
>
> Uhm, after this patch there's no CONFIG_MITIGATION_SPECTRE_BHI_ON anymore,
> which is kindof nasty, as IS_ENABLED() doesn't generate a build failure for
> non-existent Kconfig variables IIRC ...
>
> So AFAICT this patch turns on BHI unconditionally.
>
> I've fixed as per the patch below, but please double check the end result
> in tip:x86/urgent once I've pushed it out..
Oof, thanks. The result in tip looks good.
We should add a permanent build time check to catch cases like this.
--
Josh
* Ingo Molnar <mingo@kernel.org> wrote:
> > static enum bhi_mitigations bhi_mitigation __ro_after_init =
> > - IS_ENABLED(CONFIG_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
> > + IS_ENABLED(CONFIG_MITIGATION_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
>
> Uhm, after this patch there's no CONFIG_MITIGATION_SPECTRE_BHI_ON
> anymore, which is kindof nasty, as IS_ENABLED() doesn't generate a build
> failure for non-existent Kconfig variables IIRC ...
>
> So AFAICT this patch turns on BHI unconditionally.
BTW., this is why IS_ENABLED() is a bad primitive IMO:
kepler:~/tip> for N in $(git grep -w IS_ENABLED arch/x86/ | sed 's/.*IS_ENABLED(//g' | sed 's/).*//g' | sort | uniq | sed 's/^CONFIG_//g'); do printf "# %40s: " $N; git grep -E "^config $N$" -- '**Kconfig**' | wc -l; done | grep -w 0
# RESCTRL_RMID_DEPENDS_ON_CLOSID: 0
# NODE_NOT_IN_PAGE_FLAGS: 0
1)
CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID doesn't exist anymore, but is used
widely:
kepler:~/tip> git grep RESCTRL_RMID_DEPENDS_ON_CLOSID
arch/x86/kernel/cpu/resctrl/monitor.c: * Only allocated when CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID is defined.
arch/x86/kernel/cpu/resctrl/monitor.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
arch/x86/kernel/cpu/resctrl/monitor.c: if (!IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
arch/x86/kernel/cpu/resctrl/monitor.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
arch/x86/kernel/cpu/resctrl/monitor.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
arch/x86/kernel/cpu/resctrl/monitor.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
arch/x86/kernel/cpu/resctrl/monitor.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
arch/x86/kernel/cpu/resctrl/rdtgroup.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
Each of those uses is bogus, as the Kconfig symbol doesn't exist. (!)
AFAICT CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID was never defined within the
upstream kernel (!!).
AFAICT the first bogus CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID use was
introduced in this recent commit:
b30a55df60c3 ("x86/resctrl: Track the number of dirty RMID a CLOSID has")
... and was cargo-cult copied in other patches. It was never explained in
the changelog why it's used without a Kconfig entry anywhere.
Maybe in the future some other arch might (or might not) introduce
RESCTRL_RMID_DEPENDS_ON_CLOSID, but that doesn't justify this bad pattern
of dead code ...
2)
The NODE_NOT_IN_PAGE_FLAGS use is borderline correct:
kepler:~/tip> git grep -w NODE_NOT_IN_PAGE_FLAGS arch/x86
arch/x86/mm/numa.c: if (IS_ENABLED(NODE_NOT_IN_PAGE_FLAGS)) {
As NODE_NOT_IN_PAGE_FLAGS is not a Kconfig symbol, but a flag defined by
the VM:
include/linux/page-flags-layout.h:#define NODE_NOT_IN_PAGE_FLAGS 1
... which happens to work with how IS_ENABLED() is defined. No other user
of NODE_NOT_IN_PAGE_FLAGS uses the IS_ENABLED() pattern.
Thanks,
Ingo
Hi Ingo,
On 4/11/2024 1:18 AM, Ingo Molnar wrote:
>
> * Ingo Molnar <mingo@kernel.org> wrote:
>
>>> static enum bhi_mitigations bhi_mitigation __ro_after_init =
>>> - IS_ENABLED(CONFIG_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
>>> + IS_ENABLED(CONFIG_MITIGATION_SPECTRE_BHI_ON) ? BHI_MITIGATION_ON : BHI_MITIGATION_OFF;
>>
>> Uhm, after this patch there's no CONFIG_MITIGATION_SPECTRE_BHI_ON
>> anymore, which is kindof nasty, as IS_ENABLED() doesn't generate a build
>> failure for non-existent Kconfig variables IIRC ...
>>
>> So AFAICT this patch turns on BHI unconditionally.
>
> BTW., this is why IS_ENABLED() is a bad primitive IMO:
>
> kepler:~/tip> for N in $(git grep -w IS_ENABLED arch/x86/ | sed 's/.*IS_ENABLED(//g' | sed 's/).*//g' | sort | uniq | sed 's/^CONFIG_//g'); do printf "# %40s: " $N; git grep -E "^config $N$" -- '**Kconfig**' | wc -l; done | grep -w 0
> # RESCTRL_RMID_DEPENDS_ON_CLOSID: 0
> # NODE_NOT_IN_PAGE_FLAGS: 0
>
> 1)
>
> CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID doesn't exist anymore, but is used
> widely:
>
> kepler:~/tip> git grep RESCTRL_RMID_DEPENDS_ON_CLOSID
> arch/x86/kernel/cpu/resctrl/monitor.c: * Only allocated when CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID is defined.
> arch/x86/kernel/cpu/resctrl/monitor.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
> arch/x86/kernel/cpu/resctrl/monitor.c: if (!IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
> arch/x86/kernel/cpu/resctrl/monitor.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
> arch/x86/kernel/cpu/resctrl/monitor.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
> arch/x86/kernel/cpu/resctrl/monitor.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
> arch/x86/kernel/cpu/resctrl/monitor.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
> arch/x86/kernel/cpu/resctrl/rdtgroup.c: if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
>
> Each of those uses is bogus, as the Kconfig symbol doesn't exist. (!)
>
> AFAICT CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID was never defined within the
> upstream kernel (!!).
>
> AFAICT the first bogus CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID use was
> introduced in this recent commit:
>
> b30a55df60c3 ("x86/resctrl: Track the number of dirty RMID a CLOSID has")
>
> ... and was cargo-cult copied in other patches. It was never explained in
> the changelog why it's used without a Kconfig entry anywhere.
>
> Maybe in the future some other arch might (or might not) introduce
> RESCTRL_RMID_DEPENDS_ON_CLOSID, but that doesn't justify this bad pattern
> of dead code ...
>
We are in the final stage [1] of untangling the resctrl filesystem code from the
x86 specific code with the goal for the resctrl interface to also be used for Arm's
(Memory System Resource Partitioning and Monitoring) MPAM that is similar enough
to AMD's Platform Quality of Service (PQOS) and Intel's Resource Director Technology
(RDT) for which resctrl is currently used.
During this work we incrementally separated the resctrl filesystem from the x86
specific code. RESCTRL_RMID_DEPENDS_ON_CLOSID was introduced during this effort
to prepare the resctrl filesystem for a fundamental difference between x86 and Arm
and you are correct that this is dead code until the Arm support arrives. The Kconfig
entry is currently being reviewed [2] as part of the portion where the resctrl
filesystem structure starts to take shape but it remains without a user until the
Arm support arrives in the next part of this effort.
I do believe that there are precedent for subsystems obtaining support for features
a couple of kernel versions before users of the features appear. With Arm MPAM support
requiring this capability from the new "resctrl filesystem" it did seem reasonable at
the time to add the capability in preparation for Arm support.
Reinette
[1] https://lore.kernel.org/lkml/20240321165106.31602-1-james.morse@arm.com/
[2] https://lore.kernel.org/lkml/20240321165106.31602-30-james.morse@arm.com/
© 2016 - 2026 Red Hat, Inc.