Re: [PATCH v2] x86/bugs: Break down mitigations configurations

Pawan Gupta
2023-06-27T22:30:53Z

On Tue, Jun 27, 2023 at 10:36:10AM -0700, Breno Leitao wrote:
> Hello Pawan,
>
> On Wed, Jun 21, 2023 at 12:41:01PM -0700, Pawan Gupta wrote:
> > On Wed, Jun 21, 2023 at 11:36:53AM -0700, Breno Leitao wrote:
> > > If I understand where you want to go, you think we should create a
> > > single patchset that creates a CONFIG_<MITIGATION> for each mitigation,
> > > and move get it under CONFIG_SPECULATION_MITIGATIONS.
> >
> > Yes, a single series (or a patch) that adds config for each mitigation
> > would be good.
>
> I've been working on this request, and I may need your help to validate
> the wordings and dependencies (as in architecture/vendors where the
> problem needs to be mitigations) for each entry.

Kconfig text looks fine to me. (Some comments on arch/vendor dependency
are down below).

> Also, I want to make sure I am not missing anything. Here is what I have
> so far. Is it in the right direction?
>
> --
> Author: Breno Leitao <leitao@debian.org>
> Date: Thu Jun 15 08:04:16 2023 -0700
>
> x86/bugs: Break down mitigations configurations

How about this?

x86/bugs: Add a separate config for each mitigation

> Create an entry for each CPU mitigation under
> CONFIG_SPECULATION_MITIGATIONS. This allow users to enable or disable
> them at compilation time.
>
> If a mitigation is disabled at compilation time, it could be enabled at
> runtime using kernel command line arguments.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 53bab123a8ee..10ea7884eddd 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2649,6 +2649,100 @@ config SLS
> against straight line speculation. The kernel image might be slightly
> larger.
>
> +config MITIGATE_MDS
> + bool "Mitigate Microarchitectural Data Sampling (MDS) hardware bug"
> + depends on CPU_SUP_INTEL && X86_64

Architecture/vendor dependency is resolved at runtime during bug
enumeration (using CPU family/model). I don't think there is a need to
add explicit dependency here unless it creates runtime issues. And for
these configs it doesn't.

MDS and some of the other mitigations works for 32-bit kernel as well.
Dependency on X86_64 here is not correct, it makes 32-bit systems
vulnerable.

> + default y
> + help
> + Enable mitigation for Microarchitectural Data Sampling (MDS). MDS is
> + a hardware vulnerability which allows unprivileged speculative access
> + to data which is available in various CPU internal buffer. Deeper
> + technical information is available in the MDS specific x86 architecture
> + section: Documentation/arch/x86/mds.rst.
> +
> +config MITIGATE_TAA
> + bool "Mitigate TSX Asynchronous Abort (TAA) hardware bug"
> + depends on CPU_SUP_INTEL && X86_64

Ditto.

> + default y
> + help
> + Enable mitigation for TSX Asynchronous Abort (TAA). TAA is a hardware
> + vulnerability that allows unprivileged speculative access to data
> + which is available in various CPU internal buffers by using
> + asynchronous aborts within an Intel TSX transactional region.
> +
> +config MITIGATE_MMIO_STALE_DATA
> + bool "Mitigate MMIO Stale Data hardware bug"
> + depends on CPU_SUP_INTEL && X86_64

Ditto for and all the others.

[...]
> @@ -1286,7 +1316,7 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
>
> ret = cmdline_find_option(boot_command_line, "spectre_v2", arg, sizeof(arg));
> if (ret < 0)
> - return SPECTRE_V2_CMD_AUTO;
> + return cmd;

In the same function, below code also needs to return the compile time
default:

if (i >= ARRAY_SIZE(mitigation_options)) {
pr_err("unknown option (%s). Switching to AUTO select\n", arg);
return SPECTRE_V2_CMD_AUTO;
}

[...]
> @@ -2119,7 +2153,12 @@ EXPORT_SYMBOL_GPL(itlb_multihit_kvm_mitigation);
> #define pr_fmt(fmt) "L1TF: " fmt
>
> /* Default mitigation for L1TF-affected CPUs */
> +

Extra newline.

> +#if IS_ENABLED(CONFIG_MITIGATE_L1TF)
> enum l1tf_mitigations l1tf_mitigation __ro_after_init = L1TF_MITIGATION_FLUSH;
> +#else
> +enum l1tf_mitigations l1tf_mitigation __ro_after_init = L1TF_MITIGATION_OFF;
> +#endif
> #if IS_ENABLED(CONFIG_KVM_INTEL)
> EXPORT_SYMBOL_GPL(l1tf_mitigation);
> #endif