[MODERATED] Re: [patch V4 07/11] x86/speculation/mds: Add mitigation control for MDS

[mark gross <mgross@linux.intel.com>]

On Fri, Feb 22, 2019 at 11:24:25PM +0100, speck for Thomas Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Now that the mitigations are in place, add a command line parameter to
> control the mitigation, a mitigation selector function and a SMT update
> mechanism.
> 
> This is the minimal straight forward initial implementation which just
> provides an always on/off mode. The command line parameter is:
> 
>   mds=[full|off|auto]
do we need full and auto?

> 
> This is consistent with the existing mitigations for other speculative
> hardware vulnerabilities.
> 
> The idle invocation is dynamically updated according to the SMT state of
> the system similar to the dynamic update of the STIBP mitigation.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Borislav Petkov <bp@suse.de>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |   27 ++++++++
>  arch/x86/include/asm/processor.h                |    6 +
>  arch/x86/kernel/cpu/bugs.c                      |   76 ++++++++++++++++++++++++
>  3 files changed, 109 insertions(+)
> 
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2356,6 +2356,33 @@
>  			Format: <first>,<last>
>  			Specifies range of consoles to be captured by the MDA.
>  
> +	mds=		[X86,INTEL]
> +			Control mitigation for the Micro-architectural Data
> +			Sampling (MDS) vulnerability.
> +
> +			Certain CPUs are vulnerable to an exploit against CPU
> +			internal buffers which can forward information to a
> +			disclosure gadget under certain conditions.
> +
> +			In vulnerable processors, the speculatively
> +			forwarded data can be used in a cache side channel
> +			attack, to access data to which the attacker does
> +			not have direct access.
> +
> +			This parameter controls the MDS mitigation. The the
> +			options are:
> +
> +			full    - Unconditionally enable MDS mitigation
> +			off     - Unconditionally disable MDS mitigation
> +			auto    - Kernel detects whether the CPU model is
> +				  vulnerable to MDS and picks the most
> +				  appropriate mitigation. If the CPU is not
> +				  vulnerable, "off" is selected. If the CPU
> +				  is vulnerable "full" is selected.
> +
> +			Not specifying this option is equivalent to
> +			mds=auto.
> +
>  	mem=nn[KMG]	[KNL,BOOT] Force usage of a specific amount of memory
>  			Amount of memory to be used when the kernel is not able
>  			to see the whole system memory or for test.
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -992,4 +992,10 @@ enum l1tf_mitigations {
>  
>  extern enum l1tf_mitigations l1tf_mitigation;
>  
> +enum mds_mitigations {
> +	MDS_MITIGATION_OFF,
> +	MDS_MITIGATION_AUTO,
> +	MDS_MITIGATION_FULL,
> +};
> +
>  #endif /* _ASM_X86_PROCESSOR_H */
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -37,6 +37,7 @@
>  static void __init spectre_v2_select_mitigation(void);
>  static void __init ssb_select_mitigation(void);
>  static void __init l1tf_select_mitigation(void);
> +static void __init mds_select_mitigation(void);
>  
>  /* The base value of the SPEC_CTRL MSR that always has to be preserved. */
>  u64 x86_spec_ctrl_base;
> @@ -106,6 +107,8 @@ void __init check_bugs(void)
>  
>  	l1tf_select_mitigation();
>  
> +	mds_select_mitigation();
> +
>  #ifdef CONFIG_X86_32
>  	/*
>  	 * Check whether we are able to run this kernel safely on SMP.
> @@ -212,6 +215,59 @@ static void x86_amd_ssb_disable(void)
>  }
>  
>  #undef pr_fmt
> +#define pr_fmt(fmt)	"MDS: " fmt
> +
> +/* Default mitigation for L1TF-affected CPUs */
> +static enum mds_mitigations mds_mitigation __ro_after_init = MDS_MITIGATION_AUTO;
> +
> +static const char * const mds_strings[] = {
> +	[MDS_MITIGATION_OFF]	= "Vulnerable",
> +	[MDS_MITIGATION_FULL]	= "Mitigation: Clear CPU buffers"
> +};
> +
> +static void mds_select_mitigation(void)
> +{
> +	if (!boot_cpu_has_bug(X86_BUG_MDS)) {
> +		mds_mitigation = MDS_MITIGATION_OFF;
> +		return;
> +	}
> +
> +	switch (mds_mitigation) {
> +	case MDS_MITIGATION_OFF:
> +		break;
> +	case MDS_MITIGATION_AUTO:
> +	case MDS_MITIGATION_FULL:
here AUTO and FULL behave identically.

> +		if (boot_cpu_has(X86_FEATURE_MD_CLEAR)) {
> +			mds_mitigation = MDS_MITIGATION_FULL;
> +			static_branch_enable(&mds_user_clear);
> +		} else {
> +			mds_mitigation = MDS_MITIGATION_OFF;
> +		}
> +		break;
> +	}
> +	pr_info("%s\n", mds_strings[mds_mitigation]);
> +}
> +
> +static int __init mds_cmdline(char *str)
> +{
> +	if (!boot_cpu_has_bug(X86_BUG_MDS))
> +		return 0;
> +
> +	if (!str)
> +		return -EINVAL;
> +
> +	if (!strcmp(str, "off"))
> +		mds_mitigation = MDS_MITIGATION_OFF;
> +	else if (!strcmp(str, "auto"))
> +		mds_mitigation = MDS_MITIGATION_AUTO;
> +	else if (!strcmp(str, "full"))
> +		mds_mitigation = MDS_MITIGATION_FULL;
> +
> +	return 0;
> +}
> +early_param("mds", mds_cmdline);
> +
> +#undef pr_fmt
>  #define pr_fmt(fmt)     "Spectre V2 : " fmt
>  
>  static enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init =
> @@ -615,6 +671,15 @@ static void update_indir_branch_cond(voi
>  		static_branch_disable(&switch_to_cond_stibp);
>  }
>  
> +/* Update the static key controlling the MDS CPU buffer clear in idle */
> +static void update_mds_branch_idle(void)
> +{
> +	if (sched_smt_active())
> +		static_branch_enable(&mds_idle_clear);
> +	else
> +		static_branch_disable(&mds_idle_clear);
> +}
> +
>  void arch_smt_update(void)
>  {
>  	/* Enhanced IBRS implies STIBP. No update required. */
> @@ -636,6 +701,17 @@ void arch_smt_update(void)
>  		break;
>  	}
>  
> +	switch (mds_mitigation) {
> +	case MDS_MITIGATION_OFF:
> +		break;
> +	case MDS_MITIGATION_FULL:
> +		update_mds_branch_idle();
> +		break;
> +	/* Keep GCC happy */
> +	case MDS_MITIGATION_AUTO:
shouldn't there be a check to see if the platform needs to set mds_idle_clear
and call update_mds_branch_idle conditionally?

I'm not sure what the value of having both auto and full is.

--mark