Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Sender: Ingo Molnar <mingo.kernel.org@gmail.com>
Date:   Wed, 11 Oct 2023 23:59:47 +0200
From:   Ingo Molnar <mingo@kernel.org>
To:     Breno Leitao <leitao@debian.org>
Cc:     tglx@linutronix.de, bp@alien8.de, Ingo Molnar <mingo@redhat.com>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        Pawan Gupta <pawan.kumar.gupta@linux.intel.com>, leit@meta.com,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4] x86/bugs: Add a separate config for each mitigation
Message-ID: <ZSca08rnmZfkONEH@gmail.com>
References: <20231010103028.4192223-1-leitao@debian.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231010103028.4192223-1-leitao@debian.org>
Precedence: bulk


* Breno Leitao <leitao@debian.org> wrote:

> +config MITIGATE_MDS
> +	bool "Mitigate Microarchitectural Data Sampling (MDS) hardware bug"

> +config MITIGATE_TAA
> +	bool "Mitigate TSX Asynchronous Abort (TAA) hardware bug"

> +config MITIGATE_MMIO_STALE_DATA
> +	bool "Mitigate MMIO Stale Data hardware bug"

> +config MITIGATE_L1TF
> +	bool "Mitigate L1 Terminal Fault (L1TF) hardware bug"

> +config MITIGATE_RETBLEED
> +	bool "Mitigate RETBleed hardware bug"

> +config MITIGATE_SPECTRE_V1
> +	bool "Mitigate SPECTRE V1 hardware bug"

> +config MITIGATE_SPECTRE_V2
> +	bool "Mitigate SPECTRE V2 hardware bug"

> +config MITIGATE_SRBDS
> +	bool "Mitigate Special Register Buffer Data Sampling (SRBDS) hardware bug"

> +config MITIGATE_SSB
> +	bool "Mitigate Speculative Store Bypass (SSB) hardware bug"

> +#if IS_ENABLED(CONFIG_MITIGATE_RETBLEED)
>  static enum retbleed_mitigation_cmd retbleed_cmd __ro_after_init =
>  	RETBLEED_CMD_AUTO;
> +#else
> +static enum retbleed_mitigation_cmd retbleed_cmd __ro_after_init =
> +	RETBLEED_CMD_OFF;
> +#endif

1)

Yeah, so this #ifdeffery is unnecessarily ugly - we can actually assign 
integer values in the Kconfig language and use that for initialization.

Is there a reason why we wouldn't want to do something like:

	static enum retbleed_mitigation_cmd retbleed_cmd __ro_after_init = CONFIG_BOOT_DEFAULT_X86_MITIGATE_RETBLEED;

... or so?

2)

The new Kconfig namespace should probably be X86_MITIGATE_*, so that we 
don't crowd the generic kernel's Kconfig namespace.

3)

And yes, now that the rush of CPU vulnerabilities seems to be ebbing, we 
should probably consider unifying the existing hodgepodge of mitigation 
Kconfig options as well, to not build up even more technical debt.

The churn factor seems moderate:

  kepler:~/tip> git grep CONFIG_RETPOLINE | wc -l
  52

  kepler:~/tip> git grep RETHUNK | wc -l
  42

  kepler:~/tip> git grep CALL_DEPTH_TRACKING | wc -l
  24

... and since most of this code is maintained in the same tree, the usual 
arguments against churn (interfering with other Git trees) does not apply 
nearly as much.

4)

Fourth, I think we should inform users (in the boot log) when a kernel 
.config changes a mitigation default value compared from what the upstream 
kernel thinks is a suitable default.

Sometimes it can be a simple configuration mistake, or a user might have 
different opinion about the importance of a particular mitigation. Nothing 
heavy-handed, just a simple pr_info() table of changes?

Thanks,

	Ingo