[MODERATED] [PATCH v2.1 3/6] [PATCH v2.1 3/6] Patch #3

[MODERATED] [PATCH v2.1 3/6] [PATCH v2.1 3/6] Patch #3

[konrad.wilk]

336996-Speculative-Execution-Side-Channel-Mitigations.pdf defines
a new MSR (IA32_FLUSH_CMD aka 0x10B) which has similar semantics
to other MSRS defined in the document.

As such we implement a similar way to handle it as commit 1b86883
"x86/bugs: Read SPEC_CTRL MSR during boot and re-use reserved bits"
- that is  we read at boottime the FLUSH_CMD MSR and provide
an external API to retrieve this value and also to whack the L1 flush.

The semantics of this MSR is to allow "finer granularity invalidation
of caching structures than existing mechanisms like WBINVD. It will
writeback and invalidate the L1 data cache, including all cachelines
brought in by preceding instructions, without invalidating all caches
(eg. L2 or LLC). Some processors may also invalidate the first level level
instruction cache on a L1D_FLUSH command. The L1 data and
instruction caches may be shared across the logical processors of a core."

A copy of this document is available at
   https://bugzilla.kernel.org/show_bug.cgi?id=199511

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/include/asm/cpu.h       |  4 ++++
 arch/x86/include/asm/msr-index.h |  6 ++++++
 arch/x86/kernel/cpu/bugs.c       | 28 ++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index adc6cc86b062..1737d4b9ee1b 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -40,4 +40,8 @@ int mwait_usable(const struct cpuinfo_x86 *);
 unsigned int x86_family(unsigned int sig);
 unsigned int x86_model(unsigned int sig);
 unsigned int x86_stepping(unsigned int sig);
+
+extern u64 x86_flush_cmd_get(void);
+extern int x86_flush_l1d(void);
+
 #endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 68b2c3150de1..0e7517089b80 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -76,6 +76,12 @@
 						    * control required.
 						    */
 
+#define MSR_IA32_FLUSH_CMD		0x0000010b
+#define L1D_FLUSH			(1 << 0)   /*
+						    * Writeback and invalidate the
+						    * L1 data cache.
+						    */
+
 #define MSR_IA32_BBL_CR_CTL		0x00000119
 #define MSR_IA32_BBL_CR_CTL3		0x0000011e
 
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 50500cea6eba..659963bf9a3c 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -53,6 +53,13 @@ static u64 __ro_after_init x86_spec_ctrl_mask = SPEC_CTRL_IBRS;
 u64 __ro_after_init x86_amd_ls_cfg_base;
 u64 __ro_after_init x86_amd_ls_cfg_ssbd_mask;
 
+/*
+ * Our boot-time value of the IA32_FLUSH_CMD MSR. We read it once so that
+ * any writes to the IA32_FLUSH_CMD contain whatever reserved bits have been
+ * set.
+ */
+static u64 __ro_after_init x86_flush_cmd;
+
 void __init check_bugs(void)
 {
 	identify_boot_cpu();
@@ -85,6 +92,10 @@ void __init check_bugs(void)
 
 	l1tf_select_mitigation();
 
+	/* Similar to SPEC_CTRL MSR, read it to account for reserved bits. */
+	if (boot_cpu_has(X86_FEATURE_FLUSH_L1D))
+		rdmsrl(MSR_IA32_FLUSH_CMD, x86_flush_cmd);
+
 #ifdef CONFIG_X86_32
 	/*
 	 * Check whether we are able to run this kernel safely on SMP.
@@ -682,6 +693,23 @@ static void __init l1tf_select_mitigation(void)
 
 	setup_force_cpu_cap(X86_FEATURE_L1TF_PTEINV);
 }
+
+u64 x86_flush_cmd_get(void)
+{
+	return x86_flush_cmd;
+}
+EXPORT_SYMBOL_GPL(x86_flush_cmd_get);
+
+int x86_flush_l1d(void)
+{
+	if (boot_cpu_has(X86_FEATURE_FLUSH_L1D)) {
+		wrmsrl(MSR_IA32_FLUSH_CMD, x86_flush_cmd | L1D_FLUSH);
+		return 0;
+	}
+	return -ENOTSUPP;
+}
+EXPORT_SYMBOL_GPL(x86_flush_l1d);
+
 #undef pr_fmt
 
 #ifdef CONFIG_SYSFS
-- 
2.14.3

[MODERATED] Re: [PATCH v2.1 3/6] [PATCH v2.1 3/6] Patch #3

[Konrad Rzeszutek Wilk]

> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 50500cea6eba..659963bf9a3c 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -53,6 +53,13 @@ static u64 __ro_after_init x86_spec_ctrl_mask = SPEC_CTRL_IBRS;
>  u64 __ro_after_init x86_amd_ls_cfg_base;
>  u64 __ro_after_init x86_amd_ls_cfg_ssbd_mask;
>  
> +/*
> + * Our boot-time value of the IA32_FLUSH_CMD MSR. We read it once so that
> + * any writes to the IA32_FLUSH_CMD contain whatever reserved bits have been
> + * set.
> + */
> +static u64 __ro_after_init x86_flush_cmd;
> +
>  void __init check_bugs(void)
>  {
>  	identify_boot_cpu();
> @@ -85,6 +92,10 @@ void __init check_bugs(void)
>  
>  	l1tf_select_mitigation();
>  
> +	/* Similar to SPEC_CTRL MSR, read it to account for reserved bits. */
> +	if (boot_cpu_has(X86_FEATURE_FLUSH_L1D))
> +		rdmsrl(MSR_IA32_FLUSH_CMD, x86_flush_cmd);


The spec didn't mention it but the development microcode I am assuming looks to be a
write-only MSR, so I changed this to:

		(void)rdmsrl_safe(MSR_IA32_FLUSH_CMD, &x86_flush_cmd);

Re: [PATCH v2.1 3/6] [PATCH v2.1 3/6] Patch #3

[Thomas Gleixner]

On Wed, 20 Jun 2018, speck for Konrad Rzeszutek Wilk wrote:
> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > index 50500cea6eba..659963bf9a3c 100644
> > --- a/arch/x86/kernel/cpu/bugs.c
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -53,6 +53,13 @@ static u64 __ro_after_init x86_spec_ctrl_mask = SPEC_CTRL_IBRS;
> >  u64 __ro_after_init x86_amd_ls_cfg_base;
> >  u64 __ro_after_init x86_amd_ls_cfg_ssbd_mask;
> >  
> > +/*
> > + * Our boot-time value of the IA32_FLUSH_CMD MSR. We read it once so that
> > + * any writes to the IA32_FLUSH_CMD contain whatever reserved bits have been
> > + * set.
> > + */
> > +static u64 __ro_after_init x86_flush_cmd;
> > +
> >  void __init check_bugs(void)
> >  {
> >  	identify_boot_cpu();
> > @@ -85,6 +92,10 @@ void __init check_bugs(void)
> >  
> >  	l1tf_select_mitigation();
> >  
> > +	/* Similar to SPEC_CTRL MSR, read it to account for reserved bits. */
> > +	if (boot_cpu_has(X86_FEATURE_FLUSH_L1D))
> > +		rdmsrl(MSR_IA32_FLUSH_CMD, x86_flush_cmd);
> 
> 
> The spec didn't mention it but the development microcode I am assuming looks to be a
> write-only MSR, so I changed this to:
> 
> 		(void)rdmsrl_safe(MSR_IA32_FLUSH_CMD, &x86_flush_cmd);

Which is pointless as my version of that magic PDF clearly says:

  New MSR IA32_FLUSH_CMD (MSR 0x10B, Write-only) bit 0: L1D Flush

Thanks,

	tglx

[MODERATED] Re: [PATCH v2.1 3/6] [PATCH v2.1 3/6] Patch #3

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

On 20/06/2018 22:42, speck for konrad.wilk_at_oracle.com wrote:
> +	/* Similar to SPEC_CTRL MSR, read it to account for reserved bits. */
> +	if (boot_cpu_has(X86_FEATURE_FLUSH_L1D))
> +		rdmsrl(MSR_IA32_FLUSH_CMD, x86_flush_cmd);
> +
>  #ifdef CONFIG_X86_32
>  	/*
>  	 * Check whether we are able to run this kernel safely on SMP.
> @@ -682,6 +693,23 @@ static void __init l1tf_select_mitigation(void)
>  
>  	setup_force_cpu_cap(X86_FEATURE_L1TF_PTEINV);
>  }
> +
> +u64 x86_flush_cmd_get(void)
> +{
> +	return x86_flush_cmd;
> +}
> +EXPORT_SYMBOL_GPL(x86_flush_cmd_get);
> +
> +int x86_flush_l1d(void)
> +{
> +	if (boot_cpu_has(X86_FEATURE_FLUSH_L1D)) {
> +		wrmsrl(MSR_IA32_FLUSH_CMD, x86_flush_cmd | L1D_FLUSH);
> +		return 0;
> +	}

We don't do that for PRED_CMD, we shouldn't do that here either.  It's
an almost guaranteed cache miss.

Paolo

[MODERATED] Re: [PATCH v2.1 3/6] [PATCH v2.1 3/6] Patch #3

[Konrad Rzeszutek Wilk]

On Thu, Jun 21, 2018 at 09:58:32AM +0200, speck for Thomas Gleixner wrote:
> On Wed, 20 Jun 2018, speck for Konrad Rzeszutek Wilk wrote:
> > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > > index 50500cea6eba..659963bf9a3c 100644
> > > --- a/arch/x86/kernel/cpu/bugs.c
> > > +++ b/arch/x86/kernel/cpu/bugs.c
> > > @@ -53,6 +53,13 @@ static u64 __ro_after_init x86_spec_ctrl_mask = SPEC_CTRL_IBRS;
> > >  u64 __ro_after_init x86_amd_ls_cfg_base;
> > >  u64 __ro_after_init x86_amd_ls_cfg_ssbd_mask;
> > >  
> > > +/*
> > > + * Our boot-time value of the IA32_FLUSH_CMD MSR. We read it once so that
> > > + * any writes to the IA32_FLUSH_CMD contain whatever reserved bits have been
> > > + * set.
> > > + */
> > > +static u64 __ro_after_init x86_flush_cmd;
> > > +
> > >  void __init check_bugs(void)
> > >  {
> > >  	identify_boot_cpu();
> > > @@ -85,6 +92,10 @@ void __init check_bugs(void)
> > >  
> > >  	l1tf_select_mitigation();
> > >  
> > > +	/* Similar to SPEC_CTRL MSR, read it to account for reserved bits. */
> > > +	if (boot_cpu_has(X86_FEATURE_FLUSH_L1D))
> > > +		rdmsrl(MSR_IA32_FLUSH_CMD, x86_flush_cmd);
> > 
> > 
> > The spec didn't mention it but the development microcode I am assuming looks to be a
> > write-only MSR, so I changed this to:
> > 
> > 		(void)rdmsrl_safe(MSR_IA32_FLUSH_CMD, &x86_flush_cmd);
> 
> Which is pointless as my version of that magic PDF clearly says:
> 
>   New MSR IA32_FLUSH_CMD (MSR 0x10B, Write-only) bit 0: L1D Flush

Duh. Indeed. <rips it out>
> 
> Thanks,
> 
> 	tglx