[MODERATED] Re: [patch V2 09/10] MDS basics+ 9

[Borislav Petkov <bp@suse.de>]

On Wed, Feb 20, 2019 at 04:08:02PM +0100, speck for Thomas Gleixner wrote:
> To avoid the expensive CPU buffer flushing on every transition from kernel
> to user space it is desired to provide a conditional mitigation mode.
> 
> Provide the infrastructure which is required to implement this:
> 
>  - A static key to enable conditional mode CPU buffer flushing.
> 
>  - A per CPU variable which indicates that CPU buffers need to
>    be flushed on return to user space. The variable is defined
>    next to __preempt_count to it ends up in a cacheline which

			    so

>    is required on return to user space anyway.
> 
>  - The conditinal flush mechanics on return to user space.

	conditional

> 
>  - A helper function to set the flush request. Is in processor.h for now to
>    avoid include hell, but might move to a separate header.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/entry/common.c              |    6 ++++++
>  arch/x86/include/asm/nospec-branch.h |    3 +++
>  arch/x86/include/asm/processor.h     |   13 +++++++++++++
>  arch/x86/kernel/cpu/bugs.c           |    1 +
>  arch/x86/kernel/cpu/common.c         |    7 +++++++
>  5 files changed, 30 insertions(+)
> 
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -183,6 +183,12 @@ static void exit_to_usermode_loop(struct
>  
>  static inline void mds_user_clear_cpu_buffers(void)
>  {
> +	if (static_branch_likely(&mds_user_clear_cond)) {
> +		if (__this_cpu_read(mds_cond_clear)) {
> +			__this_cpu_write(mds_cond_clear, 0);
> +			mds_clear_cpu_buffers();
> +		}
> +	}
>  	if (static_branch_likely(&mds_user_clear_always))
>  		mds_clear_cpu_buffers();
>  }
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -9,6 +9,7 @@
>  #include <asm/alternative-asm.h>
>  #include <asm/cpufeatures.h>
>  #include <asm/msr-index.h>
> +#include <asm/percpu.h>
>  
>  /*
>   * Fill the CPU return stack buffer.
> @@ -319,7 +320,9 @@ DECLARE_STATIC_KEY_FALSE(switch_mm_cond_
>  DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
>  
>  DECLARE_STATIC_KEY_FALSE(mds_user_clear_always);
> +DECLARE_STATIC_KEY_FALSE(mds_user_clear_cond);
>  DECLARE_STATIC_KEY_FALSE(mds_idle_clear);
> +DECLARE_PER_CPU(unsigned int, mds_cond_clear);
>  
>  #include <asm/segment.h>
>  
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -24,6 +24,7 @@ struct vm86;
>  #include <asm/special_insns.h>
>  #include <asm/fpu/types.h>
>  #include <asm/unwind_hints.h>
> +#include <asm/nospec-branch.h>
>  
>  #include <linux/personality.h>
>  #include <linux/cache.h>
> @@ -998,4 +999,16 @@ enum mds_mitigations {
>  	MDS_MITIGATION_HOPE,
>  };
>  
> +/**
> + * mds_request_buffer_clear - Set the request to clear CPU buffers
> + *
> + * This is invoked from contexts which identify a necessarity to clear CPU
							^
							necessity

> + * buffers on the next return to user space.
> + */
> +static inline void mds_request_buffer_clear(void)
> +{
> +	if (static_branch_likely(&mds_user_clear_cond))
> +		this_cpu_write(mds_cond_clear, 1);
> +}
> +
>  #endif /* _ASM_X86_PROCESSOR_H */
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -66,6 +66,7 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_always
>  
>  /* Control MDS CPU buffer clear before returning to user space */
>  DEFINE_STATIC_KEY_FALSE(mds_user_clear_always);
> +DEFINE_STATIC_KEY_FALSE(mds_user_clear_cond);
>  /* Control MDS CPU buffer clear before idling (halt, mwait) */
>  DEFINE_STATIC_KEY_FALSE(mds_idle_clear);
>  
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -8,6 +8,7 @@
>  #include <linux/export.h>
>  #include <linux/percpu.h>
>  #include <linux/string.h>
> +#include <linux/nospec.h>
>  #include <linux/ctype.h>
>  #include <linux/delay.h>
>  #include <linux/sched/mm.h>
> @@ -1544,6 +1545,9 @@ DEFINE_PER_CPU(unsigned int, irq_count)
>  DEFINE_PER_CPU(int, __preempt_count) = INIT_PREEMPT_COUNT;
>  EXPORT_PER_CPU_SYMBOL(__preempt_count);
>  
> +/* Indicator for return to user space or VMENTER to clear CPU buffers */
> +DEFINE_PER_CPU(unsigned int, mds_cond_clear);
> +
>  /* May not be marked __init: used by software suspend */
>  void syscall_init(void)
>  {
> @@ -1617,6 +1621,9 @@ EXPORT_PER_CPU_SYMBOL(current_task);
>  DEFINE_PER_CPU(int, __preempt_count) = INIT_PREEMPT_COUNT;
>  EXPORT_PER_CPU_SYMBOL(__preempt_count);
>  
> +/* Indicator for return to user space or VMENTER to clear CPU buffers */
> +DEFINE_PER_CPU(unsigned int, mds_cond_clear);
> +

Why not define it once in the bitness-agnostic place over #ifdef CONFIG_X86_64?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--