All of lore.kernel.org
 help / color / mirror / Atom feed
* [MODERATED] [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3
@ 2018-04-23 17:11 konrad.wilk
  2018-04-23 18:01 ` [MODERATED] " Borislav Petkov
  0 siblings, 1 reply; 10+ messages in thread
From: konrad.wilk @ 2018-04-23 17:11 UTC (permalink / raw)
  To: speck

The 336996-Speculative-Execution-Side-Channel-Mitigations.pdf
refers to all the other bits as reserved. The Intel SDM glossary
defines reserved as implementation specific - aka unknown.

As such at bootup we should take it in-to account and apply
proper masking for the bits we use.

XXSuggested-by: Jon Masters <jcm@redhat.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v2: New patch
v3: Expose only one global variable
---
 arch/x86/include/asm/nospec-branch.h | 21 +++++++++++++++++----
 arch/x86/kernel/cpu/bugs.c           | 15 +++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index f928ad9b143f..5e441b54d8dc 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -217,6 +217,16 @@ enum spectre_v2_mitigation {
 	SPECTRE_V2_IBRS,
 };
 
+/*
+ * The Intel specification for the SPEC_CTRL MSR requires that we
+ * preserve any already set reserved bits at boot time (e.g. for
+ * future additions that this kernel is not currently aware of).
+ * We then set any additional mitigation bits that we want
+ * ourselves and always use this as the base for SPEC_CTRL.
+ * We also use this when handling guest entry/exit as below.
+ */
+extern u64 x86_spec_ctrl_base;
+
 extern char __indirect_thunk_start[];
 extern char __indirect_thunk_end[];
 
@@ -248,12 +258,13 @@ static inline void vmexit_fill_RSB(void)
 				 "movl $0, %%edx\n\t"		\
 				 "wrmsr",			\
 				 _feature)			\
-		     : : [msr] "i" (_msr), [val] "i" (_val)	\
+		     : : [msr] "i" (_msr), [val] "m" (_val)	\
 		     : "eax", "ecx", "edx", "memory")
 
 static inline void indirect_branch_prediction_barrier(void)
 {
-	alternative_msr_write(MSR_IA32_PRED_CMD, PRED_CMD_IBPB,
+	u64 val = PRED_CMD_IBPB;
+	alternative_msr_write(MSR_IA32_PRED_CMD, val,
 			      X86_FEATURE_USE_IBPB);
 }
 
@@ -265,14 +276,16 @@ static inline void indirect_branch_prediction_barrier(void)
  */
 #define firmware_restrict_branch_speculation_start()			\
 do {									\
+	u64 val = x86_spec_ctrl_base | SPEC_CTRL_IBRS;			\
 	preempt_disable();						\
-	alternative_msr_write(MSR_IA32_SPEC_CTRL, SPEC_CTRL_IBRS,	\
+	alternative_msr_write(MSR_IA32_SPEC_CTRL, val,			\
 			      X86_FEATURE_USE_IBRS_FW);			\
 } while (0)
 
 #define firmware_restrict_branch_speculation_end()			\
 do {									\
-	alternative_msr_write(MSR_IA32_SPEC_CTRL, 0,			\
+	alternative_msr_write(MSR_IA32_SPEC_CTRL,			\
+			      x86_spec_ctrl_base,			\
 			      X86_FEATURE_USE_IBRS_FW);			\
 	preempt_enable();						\
 } while (0)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index ad613f73d071..bd48686e5c1a 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -27,11 +27,14 @@
 #include <asm/intel-family.h>
 
 static void __init spectre_v2_select_mitigation(void);
+static void __init spec_ctrl_save_msr(void);
 
 void __init check_bugs(void)
 {
 	identify_boot_cpu();
 
+	spec_ctrl_save_msr();
+
 	if (!IS_ENABLED(CONFIG_SMP)) {
 		pr_info("CPU: ");
 		print_cpu_info(&boot_cpu_data);
@@ -95,6 +98,18 @@ static const char *spectre_v2_strings[] = {
 
 static enum spectre_v2_mitigation spectre_v2_enabled = SPECTRE_V2_NONE;
 
+u64 x86_spec_ctrl_base;
+
+static void __init spec_ctrl_save_msr(void)
+{
+	/*
+	 * Read the SPEC_CTRL MSR to account for reserved bits which may have
+	 * unknown values.
+	 */
+	if (boot_cpu_has(X86_FEATURE_IBPB) || boot_cpu_has(X86_FEATURE_IBRS))
+		rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+}
+
 #ifdef RETPOLINE
 static bool spectre_v2_bad_module;
 
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [MODERATED] Re: [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3
  2018-04-23 17:11 [MODERATED] [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3 konrad.wilk
@ 2018-04-23 18:01 ` Borislav Petkov
  2018-04-23 23:49   ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2018-04-23 18:01 UTC (permalink / raw)
  To: speck

On Mon, Apr 23, 2018 at 01:11:27PM -0400, speck for konrad.wilk_at_oracle.com wrote:
> x86/spectre_v2: Read SPEC_CTRL MSR during boot and re-use reserved bits.

"x86/bugs: ..."


> The 336996-Speculative-Execution-Side-Channel-Mitigations.pdf

Those documents tend to disappear from vendor pages after a while so
what we do is we open a bugzilla entry on bugzilla.kernel.org, upload
the doc there and then refer to it from the commit message.

> refers to all the other bits as reserved. The Intel SDM glossary
> defines reserved as implementation specific - aka unknown.
> 
> As such at bootup we should take it in-to account and apply

s/in-to/into/

> proper masking for the bits we use.
> 
> XXSuggested-by: Jon Masters <jcm@redhat.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> v2: New patch
> v3: Expose only one global variable
> ---
>  arch/x86/include/asm/nospec-branch.h | 21 +++++++++++++++++----
>  arch/x86/kernel/cpu/bugs.c           | 15 +++++++++++++++
>  2 files changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index f928ad9b143f..5e441b54d8dc 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -217,6 +217,16 @@ enum spectre_v2_mitigation {
>  	SPECTRE_V2_IBRS,
>  };
>  
> +/*
> + * The Intel specification for the SPEC_CTRL MSR requires that we
> + * preserve any already set reserved bits at boot time (e.g. for
> + * future additions that this kernel is not currently aware of).
> + * We then set any additional mitigation bits that we want
> + * ourselves and always use this as the base for SPEC_CTRL.
> + * We also use this when handling guest entry/exit as below.
> + */

Nice commenting.

> +extern u64 x86_spec_ctrl_base;

I don't like us exporting an MSR value which other code can then change.
What happened to

	    x86_set_spec_ctrl(u64)
	u64 x86_get_spec_ctrl(void)

with the setter performing all kinds of sanity checks on the value?

> +
>  extern char __indirect_thunk_start[];
>  extern char __indirect_thunk_end[];
>  
> @@ -248,12 +258,13 @@ static inline void vmexit_fill_RSB(void)
>  				 "movl $0, %%edx\n\t"		\
>  				 "wrmsr",			\
>  				 _feature)			\
> -		     : : [msr] "i" (_msr), [val] "i" (_val)	\
> +		     : : [msr] "i" (_msr), [val] "m" (_val)	\
>  		     : "eax", "ecx", "edx", "memory")
>  
>  static inline void indirect_branch_prediction_barrier(void)
>  {
> -	alternative_msr_write(MSR_IA32_PRED_CMD, PRED_CMD_IBPB,
> +	u64 val = PRED_CMD_IBPB;

<---- newline here.

> +	alternative_msr_write(MSR_IA32_PRED_CMD, val,
>  			      X86_FEATURE_USE_IBPB);
>  }
>  
> @@ -265,14 +276,16 @@ static inline void indirect_branch_prediction_barrier(void)
>   */
>  #define firmware_restrict_branch_speculation_start()			\
>  do {									\
> +	u64 val = x86_spec_ctrl_base | SPEC_CTRL_IBRS;			\

<---- newline here.

>  	preempt_disable();						\
> -	alternative_msr_write(MSR_IA32_SPEC_CTRL, SPEC_CTRL_IBRS,	\
> +	alternative_msr_write(MSR_IA32_SPEC_CTRL, val,			\
>  			      X86_FEATURE_USE_IBRS_FW);			\
>  } while (0)
>  
>  #define firmware_restrict_branch_speculation_end()			\
>  do {									\
> -	alternative_msr_write(MSR_IA32_SPEC_CTRL, 0,			\
> +	alternative_msr_write(MSR_IA32_SPEC_CTRL,			\
> +			      x86_spec_ctrl_base,			\
>  			      X86_FEATURE_USE_IBRS_FW);			\
>  	preempt_enable();						\
>  } while (0)

...

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [MODERATED] Re: [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3
  2018-04-23 18:01 ` [MODERATED] " Borislav Petkov
@ 2018-04-23 23:49   ` Konrad Rzeszutek Wilk
  2018-04-24  8:30     ` Borislav Petkov
  0 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2018-04-23 23:49 UTC (permalink / raw)
  To: speck

On Mon, Apr 23, 2018 at 08:01:05PM +0200, speck for Borislav Petkov wrote:
> On Mon, Apr 23, 2018 at 01:11:27PM -0400, speck for konrad.wilk_at_oracle.com wrote:
> > x86/spectre_v2: Read SPEC_CTRL MSR during boot and re-use reserved bits.
> 
> "x86/bugs: ..."
> 
> 
> > The 336996-Speculative-Execution-Side-Channel-Mitigations.pdf
> 
> Those documents tend to disappear from vendor pages after a while so
> what we do is we open a bugzilla entry on bugzilla.kernel.org, upload
> the doc there and then refer to it from the commit message.

.. after the embargo? Bit of a chicken and egg problem here.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [MODERATED] Re: [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3
  2018-04-23 23:49   ` Konrad Rzeszutek Wilk
@ 2018-04-24  8:30     ` Borislav Petkov
  2018-04-24 15:27       ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2018-04-24  8:30 UTC (permalink / raw)
  To: speck

On Mon, Apr 23, 2018 at 07:49:48PM -0400, speck for Konrad Rzeszutek Wilk wrote:
> .. after the embargo? Bit of a chicken and egg problem here.

I'm afraid I don't understand what you mean here. That document is
public:

https://software.intel.com/sites/default/files/managed/c5/63/336996-Speculative-Execution-Side-Channel-Mitigations.pdf

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [MODERATED] Re: [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3
  2018-04-24  8:30     ` Borislav Petkov
@ 2018-04-24 15:27       ` Konrad Rzeszutek Wilk
  2018-04-24 15:39         ` Borislav Petkov
  2018-04-24 16:15         ` Andi Kleen
  0 siblings, 2 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2018-04-24 15:27 UTC (permalink / raw)
  To: speck

On Tue, Apr 24, 2018 at 10:30:43AM +0200, speck for Borislav Petkov wrote:
> On Mon, Apr 23, 2018 at 07:49:48PM -0400, speck for Konrad Rzeszutek Wilk wrote:
> > .. after the embargo? Bit of a chicken and egg problem here.
> 
> I'm afraid I don't understand what you mean here. That document is
> public:
> 
> https://software.intel.com/sites/default/files/managed/c5/63/336996-Speculative-Execution-Side-Channel-Mitigations.pdf

I mean in terms of creating the bug and referencing it. Or can I just
open a nonsense bug with this title:

"336996-Speculative-Execution-Side-Channel-Mitigations repository"

and just explain that I want to have the document in there?

Is that common practice?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [MODERATED] Re: [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3
  2018-04-24 15:27       ` Konrad Rzeszutek Wilk
@ 2018-04-24 15:39         ` Borislav Petkov
  2018-04-24 16:15         ` Andi Kleen
  1 sibling, 0 replies; 10+ messages in thread
From: Borislav Petkov @ 2018-04-24 15:39 UTC (permalink / raw)
  To: speck

On Tue, Apr 24, 2018 at 11:27:50AM -0400, speck for Konrad Rzeszutek Wilk wrote:
> On Tue, Apr 24, 2018 at 10:30:43AM +0200, speck for Borislav Petkov wrote:
> > On Mon, Apr 23, 2018 at 07:49:48PM -0400, speck for Konrad Rzeszutek Wilk wrote:
> > > .. after the embargo? Bit of a chicken and egg problem here.
> > 
> > I'm afraid I don't understand what you mean here. That document is
> > public:
> > 
> > https://software.intel.com/sites/default/files/managed/c5/63/336996-Speculative-Execution-Side-Channel-Mitigations.pdf
> 
> I mean in terms of creating the bug and referencing it. Or can I just
> open a nonsense bug with this title:
> 
> "336996-Speculative-Execution-Side-Channel-Mitigations repository"
> 
> and just explain that I want to have the document in there?
> 
> Is that common practice?

We have done it a couple of times with vendor stuff. See

  c128dbfa0f87 ("x86/cpufeatures: Enable new SSE/AVX/AVX512 CPU features")

for an example.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [MODERATED] Re: [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3
  2018-04-24 15:27       ` Konrad Rzeszutek Wilk
  2018-04-24 15:39         ` Borislav Petkov
@ 2018-04-24 16:15         ` Andi Kleen
  2018-04-24 16:27           ` Thomas Gleixner
  1 sibling, 1 reply; 10+ messages in thread
From: Andi Kleen @ 2018-04-24 16:15 UTC (permalink / raw)
  To: speck

On Tue, Apr 24, 2018 at 11:27:50AM -0400, speck for Konrad Rzeszutek Wilk wrote:
> On Tue, Apr 24, 2018 at 10:30:43AM +0200, speck for Borislav Petkov wrote:
> > On Mon, Apr 23, 2018 at 07:49:48PM -0400, speck for Konrad Rzeszutek Wilk wrote:
> > > .. after the embargo? Bit of a chicken and egg problem here.
> > 
> > I'm afraid I don't understand what you mean here. That document is
> > public:
> > 
> > https://software.intel.com/sites/default/files/managed/c5/63/336996-Speculative-Execution-Side-Channel-Mitigations.pdf
> 
> I mean in terms of creating the bug and referencing it. Or can I just
> open a nonsense bug with this title:
> 
> "336996-Speculative-Execution-Side-Channel-Mitigations repository"
> 
> and just explain that I want to have the document in there?
> 
> Is that common practice?

No it's not. Even if the document URL changes you can usually still
google it. Just don't bother.

-Andi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3
  2018-04-24 16:15         ` Andi Kleen
@ 2018-04-24 16:27           ` Thomas Gleixner
  2018-04-25 15:52             ` [MODERATED] " Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2018-04-24 16:27 UTC (permalink / raw)
  To: speck

On Tue, 24 Apr 2018, speck for Andi Kleen wrote:
> On Tue, Apr 24, 2018 at 11:27:50AM -0400, speck for Konrad Rzeszutek Wilk wrote:
> > On Tue, Apr 24, 2018 at 10:30:43AM +0200, speck for Borislav Petkov wrote:
> > > On Mon, Apr 23, 2018 at 07:49:48PM -0400, speck for Konrad Rzeszutek Wilk wrote:
> > > > .. after the embargo? Bit of a chicken and egg problem here.
> > > 
> > > I'm afraid I don't understand what you mean here. That document is
> > > public:
> > > 
> > > https://software.intel.com/sites/default/files/managed/c5/63/336996-Speculative-Execution-Side-Channel-Mitigations.pdf
> > 
> > I mean in terms of creating the bug and referencing it. Or can I just
> > open a nonsense bug with this title:
> > 
> > "336996-Speculative-Execution-Side-Channel-Mitigations repository"
> > 
> > and just explain that I want to have the document in there?
> > 
> > Is that common practice?
> 
> No it's not. Even if the document URL changes you can usually still
> google it. Just don't bother.

We've had cases where google was not able to get vanished stuff and that's
why I asked for having permanent storage for such kind of documents.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [MODERATED] Re: [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3
  2018-04-24 16:27           ` Thomas Gleixner
@ 2018-04-25 15:52             ` Konrad Rzeszutek Wilk
  2018-04-25 18:21               ` Thomas Gleixner
  0 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2018-04-25 15:52 UTC (permalink / raw)
  To: speck

On Tue, Apr 24, 2018 at 06:27:04PM +0200, speck for Thomas Gleixner wrote:
> On Tue, 24 Apr 2018, speck for Andi Kleen wrote:
> > On Tue, Apr 24, 2018 at 11:27:50AM -0400, speck for Konrad Rzeszutek Wilk wrote:
> > > On Tue, Apr 24, 2018 at 10:30:43AM +0200, speck for Borislav Petkov wrote:
> > > > On Mon, Apr 23, 2018 at 07:49:48PM -0400, speck for Konrad Rzeszutek Wilk wrote:
> > > > > .. after the embargo? Bit of a chicken and egg problem here.
> > > > 
> > > > I'm afraid I don't understand what you mean here. That document is
> > > > public:
> > > > 
> > > > https://software.intel.com/sites/default/files/managed/c5/63/336996-Speculative-Execution-Side-Channel-Mitigations.pdf
> > > 
> > > I mean in terms of creating the bug and referencing it. Or can I just
> > > open a nonsense bug with this title:
> > > 
> > > "336996-Speculative-Execution-Side-Channel-Mitigations repository"
> > > 
> > > and just explain that I want to have the document in there?
> > > 
> > > Is that common practice?
> > 
> > No it's not. Even if the document URL changes you can usually still
> > google it. Just don't bother.
> 
> We've had cases where google was not able to get vanished stuff and that's
> why I asked for having permanent storage for such kind of documents.


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

> 
> Thanks,
> 
> 	tglx
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3
  2018-04-25 15:52             ` [MODERATED] " Konrad Rzeszutek Wilk
@ 2018-04-25 18:21               ` Thomas Gleixner
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Gleixner @ 2018-04-25 18:21 UTC (permalink / raw)
  To: speck

On Wed, 25 Apr 2018, speck for Konrad Rzeszutek Wilk wrote:
> On Tue, Apr 24, 2018 at 06:27:04PM +0200, speck for Thomas Gleixner wrote:
> > We've had cases where google was not able to get vanished stuff and that's
> > why I asked for having permanent storage for such kind of documents.
> 
> 
> A copy of this document is available at
> https://bugzilla.kernel.org/show_bug.cgi?id=199511

Thanks Konrad!

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-04-25 18:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-23 17:11 [MODERATED] [PATCH v3 03/10] [PATCH v3 3/9] Linux Patch #3 konrad.wilk
2018-04-23 18:01 ` [MODERATED] " Borislav Petkov
2018-04-23 23:49   ` Konrad Rzeszutek Wilk
2018-04-24  8:30     ` Borislav Petkov
2018-04-24 15:27       ` Konrad Rzeszutek Wilk
2018-04-24 15:39         ` Borislav Petkov
2018-04-24 16:15         ` Andi Kleen
2018-04-24 16:27           ` Thomas Gleixner
2018-04-25 15:52             ` [MODERATED] " Konrad Rzeszutek Wilk
2018-04-25 18:21               ` Thomas Gleixner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.