All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] x86/cpu: Use SERIALIZE in sync_core() when available
@ 2020-08-07  3:28 Ricardo Neri
  2020-08-17 15:47 ` [tip: x86/cpu] " tip-bot2 for Ricardo Neri
  0 siblings, 1 reply; 6+ messages in thread
From: Ricardo Neri @ 2020-08-07  3:28 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Andy Lutomirski,
	x86, Peter Zijlstra (Intel)
  Cc: Dave Hansen, Tony Luck, Cathy Zhang, Fenghua Yu, H. Peter Anvin,
	Kyung Min Park, Ravi V. Shankar, Sean Christopherson,
	linux-kernel, Ricardo Neri, Ricardo Neri, Dave Hansen,
	linux-edac

The SERIALIZE instruction gives software a way to force the processor to
complete all modifications to flags, registers and memory from previous
instructions and drain all buffered writes to memory before the next
instruction is fetched and executed. Thus, it serves the purpose of
sync_core(). Use it when available.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Cathy Zhang <cathy.zhang@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Kyung Min Park <kyung.min.park@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: linux-edac@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Tony Luck <tony.luck@intel.com>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
This is v4 from my three previous submission [1], [2], and [3]. The first
three patches of the series have been merged in Linus' tree. Hence, I am
submitting only this patch for review.

[1]. https://lkml.org/lkml/2020/7/27/8
[2]. https://lkml.org/lkml/2020/8/4/1090
[3]. https://lkml.org/lkml/2020/8/6/808

Changes since v3:
 * Reworked comments in sync_core() for better readability. (Dave Hansen)
 * Reworked the implementation to align with the style in special_insns.h.
   No functional changes were introduced. (Tony Luck)

Changes since v2:
 * Support serialize with static_cpu_has() instead of using alternative
   runtime patching directly. (Borislav Petkov)

Changes since v1:
 * Support SERIALIZE using alternative runtime patching.
   (Peter Zijlstra, H. Peter Anvin)
 * Added a note to specify which version of binutils supports SERIALIZE.
   (Peter Zijlstra)
 * Verified that (::: "memory") is used. (H. Peter Anvin)
---
 arch/x86/include/asm/special_insns.h |  6 ++++++
 arch/x86/include/asm/sync_core.h     | 26 ++++++++++++++++++--------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 59a3e13204c3..5999b0b3dd4a 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -234,6 +234,12 @@ static inline void clwb(volatile void *__p)
 
 #define nop() asm volatile ("nop")
 
+static inline void serialize(void)
+{
+	/* Instruction opcode for SERIALIZE; supported in binutils >= 2.35. */
+	asm volatile(".byte 0xf, 0x1, 0xe8" ::: "memory");
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_SPECIAL_INSNS_H */
diff --git a/arch/x86/include/asm/sync_core.h b/arch/x86/include/asm/sync_core.h
index fdb5b356e59b..089712777fd9 100644
--- a/arch/x86/include/asm/sync_core.h
+++ b/arch/x86/include/asm/sync_core.h
@@ -5,6 +5,7 @@
 #include <linux/preempt.h>
 #include <asm/processor.h>
 #include <asm/cpufeature.h>
+#include <asm/special_insns.h>
 
 #ifdef CONFIG_X86_32
 static inline void iret_to_self(void)
@@ -54,14 +55,23 @@ static inline void iret_to_self(void)
 static inline void sync_core(void)
 {
 	/*
-	 * There are quite a few ways to do this.  IRET-to-self is nice
-	 * because it works on every CPU, at any CPL (so it's compatible
-	 * with paravirtualization), and it never exits to a hypervisor.
-	 * The only down sides are that it's a bit slow (it seems to be
-	 * a bit more than 2x slower than the fastest options) and that
-	 * it unmasks NMIs.  The "push %cs" is needed because, in
-	 * paravirtual environments, __KERNEL_CS may not be a valid CS
-	 * value when we do IRET directly.
+	 * The SERIALIZE instruction is the most straightforward way to
+	 * do this but it not universally available.
+	 */
+	if (static_cpu_has(X86_FEATURE_SERIALIZE)) {
+		serialize();
+		return;
+	}
+
+	/*
+	 * For all other processors, there are quite a few ways to do this
+	 * IRET-to-self is nice because it works on every CPU, at any CPL
+	 * (so it's compatible with paravirtualization), and it never exits
+	 * to a hypervisor. The only down sides are that it's a bit slow
+	 * (it seems to be a bit more than 2x slower than the fastest
+	 * options) and that it unmasks NMIs.  The "push %cs" is needed
+	 * because, in paravirtual environments, __KERNEL_CS may not be a
+	 * valid CS value when we do IRET directly.
 	 *
 	 * In case NMI unmasking or performance ever becomes a problem,
 	 * the next best option appears to be MOV-to-CR2 and an
-- 
2.17.1


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

* [tip: x86/cpu] x86/cpu: Use SERIALIZE in sync_core() when available
  2020-08-07  3:28 [PATCH v4] x86/cpu: Use SERIALIZE in sync_core() when available Ricardo Neri
@ 2020-08-17 15:47 ` tip-bot2 for Ricardo Neri
  2020-08-18  5:31   ` [PATCH] x86/cpu: Fix typos and improve the comments in sync_core() Ingo Molnar
  0 siblings, 1 reply; 6+ messages in thread
From: tip-bot2 for Ricardo Neri @ 2020-08-17 15:47 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Andy Lutomirski, Ricardo Neri, Borislav Petkov, Tony Luck, x86, LKML

The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     bf9c912f9a649776c2d741310486a6984edaac72
Gitweb:        https://git.kernel.org/tip/bf9c912f9a649776c2d741310486a6984edaac72
Author:        Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
AuthorDate:    Thu, 06 Aug 2020 20:28:33 -07:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Mon, 17 Aug 2020 17:23:04 +02:00

x86/cpu: Use SERIALIZE in sync_core() when available

The SERIALIZE instruction gives software a way to force the processor to
complete all modifications to flags, registers and memory from previous
instructions and drain all buffered writes to memory before the next
instruction is fetched and executed. Thus, it serves the purpose of
sync_core(). Use it when available.

Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Link: https://lkml.kernel.org/r/20200807032833.17484-1-ricardo.neri-calderon@linux.intel.com
---
 arch/x86/include/asm/special_insns.h |  6 ++++++
 arch/x86/include/asm/sync_core.h     | 26 ++++++++++++++++++--------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 59a3e13..5999b0b 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -234,6 +234,12 @@ static inline void clwb(volatile void *__p)
 
 #define nop() asm volatile ("nop")
 
+static inline void serialize(void)
+{
+	/* Instruction opcode for SERIALIZE; supported in binutils >= 2.35. */
+	asm volatile(".byte 0xf, 0x1, 0xe8" ::: "memory");
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_SPECIAL_INSNS_H */
diff --git a/arch/x86/include/asm/sync_core.h b/arch/x86/include/asm/sync_core.h
index fdb5b35..4631c0f 100644
--- a/arch/x86/include/asm/sync_core.h
+++ b/arch/x86/include/asm/sync_core.h
@@ -5,6 +5,7 @@
 #include <linux/preempt.h>
 #include <asm/processor.h>
 #include <asm/cpufeature.h>
+#include <asm/special_insns.h>
 
 #ifdef CONFIG_X86_32
 static inline void iret_to_self(void)
@@ -54,14 +55,23 @@ static inline void iret_to_self(void)
 static inline void sync_core(void)
 {
 	/*
-	 * There are quite a few ways to do this.  IRET-to-self is nice
-	 * because it works on every CPU, at any CPL (so it's compatible
-	 * with paravirtualization), and it never exits to a hypervisor.
-	 * The only down sides are that it's a bit slow (it seems to be
-	 * a bit more than 2x slower than the fastest options) and that
-	 * it unmasks NMIs.  The "push %cs" is needed because, in
-	 * paravirtual environments, __KERNEL_CS may not be a valid CS
-	 * value when we do IRET directly.
+	 * The SERIALIZE instruction is the most straightforward way to
+	 * do this but it not universally available.
+	 */
+	if (static_cpu_has(X86_FEATURE_SERIALIZE)) {
+		serialize();
+		return;
+	}
+
+	/*
+	 * For all other processors, there are quite a few ways to do this.
+	 * IRET-to-self is nice because it works on every CPU, at any CPL
+	 * (so it's compatible with paravirtualization), and it never exits
+	 * to a hypervisor. The only down sides are that it's a bit slow
+	 * (it seems to be a bit more than 2x slower than the fastest
+	 * options) and that it unmasks NMIs.  The "push %cs" is needed
+	 * because, in paravirtual environments, __KERNEL_CS may not be a
+	 * valid CS value when we do IRET directly.
 	 *
 	 * In case NMI unmasking or performance ever becomes a problem,
 	 * the next best option appears to be MOV-to-CR2 and an

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

* [PATCH] x86/cpu: Fix typos and improve the comments in sync_core()
  2020-08-17 15:47 ` [tip: x86/cpu] " tip-bot2 for Ricardo Neri
@ 2020-08-18  5:31   ` Ingo Molnar
  2020-08-19  1:00     ` Ricardo Neri
  2020-08-19  7:59     ` [tip: x86/cpu] " tip-bot2 for Ingo Molnar
  0 siblings, 2 replies; 6+ messages in thread
From: Ingo Molnar @ 2020-08-18  5:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-tip-commits, Andy Lutomirski, Ricardo Neri,
	Borislav Petkov, Tony Luck, x86


* tip-bot2 for Ricardo Neri <tip-bot2@linutronix.de> wrote:

> --- a/arch/x86/include/asm/sync_core.h
> +++ b/arch/x86/include/asm/sync_core.h
> @@ -5,6 +5,7 @@
>  #include <linux/preempt.h>
>  #include <asm/processor.h>
>  #include <asm/cpufeature.h>
> +#include <asm/special_insns.h>
>  
>  #ifdef CONFIG_X86_32
>  static inline void iret_to_self(void)
> @@ -54,14 +55,23 @@ static inline void iret_to_self(void)
>  static inline void sync_core(void)
>  {
>  	/*
> +	 * The SERIALIZE instruction is the most straightforward way to
> +	 * do this but it not universally available.
> +	 */
> +	if (static_cpu_has(X86_FEATURE_SERIALIZE)) {
> +		serialize();
> +		return;
> +	}
> +
> +	/*
> +	 * For all other processors, there are quite a few ways to do this.
> +	 * IRET-to-self is nice because it works on every CPU, at any CPL
> +	 * (so it's compatible with paravirtualization), and it never exits
> +	 * to a hypervisor. The only down sides are that it's a bit slow
> +	 * (it seems to be a bit more than 2x slower than the fastest
> +	 * options) and that it unmasks NMIs.  The "push %cs" is needed
> +	 * because, in paravirtual environments, __KERNEL_CS may not be a
> +	 * valid CS value when we do IRET directly.

So there's two typos in the new comments, there are at least two 
misapplied commas, it departs from existing style, and there's a typo 
in the existing comments as well.

Also, before this patch the 'compiler barrier' comment was valid for 
the whole function (there was no branching), but after this patch it 
reads of it was only valid for the legacy IRET-to-self branch.

Which together broke my detector and triggered a bit of compulsive 
bike-shed painting. ;-) See the resulting patch below.

Thanks,

	Ingo

================>
From: Ingo Molnar <mingo@kernel.org>
Date: Tue, 18 Aug 2020 07:24:05 +0200
Subject: [PATCH] x86/cpu: Fix typos and improve the comments in sync_core()

- Fix typos.

- Move the compiler barrier comment to the top, because it's valid for the
  whole function, not just the legacy branch.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/sync_core.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/sync_core.h b/arch/x86/include/asm/sync_core.h
index 4631c0f969d4..0fd4a9dfb29c 100644
--- a/arch/x86/include/asm/sync_core.h
+++ b/arch/x86/include/asm/sync_core.h
@@ -47,16 +47,19 @@ static inline void iret_to_self(void)
  *
  *  b) Text was modified on a different CPU, may subsequently be
  *     executed on this CPU, and you want to make sure the new version
- *     gets executed.  This generally means you're calling this in a IPI.
+ *     gets executed.  This generally means you're calling this in an IPI.
  *
  * If you're calling this for a different reason, you're probably doing
  * it wrong.
+ *
+ * Like all of Linux's memory ordering operations, this is a
+ * compiler barrier as well.
  */
 static inline void sync_core(void)
 {
 	/*
 	 * The SERIALIZE instruction is the most straightforward way to
-	 * do this but it not universally available.
+	 * do this, but it is not universally available.
 	 */
 	if (static_cpu_has(X86_FEATURE_SERIALIZE)) {
 		serialize();
@@ -67,10 +70,10 @@ static inline void sync_core(void)
 	 * For all other processors, there are quite a few ways to do this.
 	 * IRET-to-self is nice because it works on every CPU, at any CPL
 	 * (so it's compatible with paravirtualization), and it never exits
-	 * to a hypervisor. The only down sides are that it's a bit slow
+	 * to a hypervisor.  The only downsides are that it's a bit slow
 	 * (it seems to be a bit more than 2x slower than the fastest
-	 * options) and that it unmasks NMIs.  The "push %cs" is needed
-	 * because, in paravirtual environments, __KERNEL_CS may not be a
+	 * options) and that it unmasks NMIs.  The "push %cs" is needed,
+	 * because in paravirtual environments __KERNEL_CS may not be a
 	 * valid CS value when we do IRET directly.
 	 *
 	 * In case NMI unmasking or performance ever becomes a problem,
@@ -81,9 +84,6 @@ static inline void sync_core(void)
 	 * CPUID is the conventional way, but it's nasty: it doesn't
 	 * exist on some 486-like CPUs, and it usually exits to a
 	 * hypervisor.
-	 *
-	 * Like all of Linux's memory ordering operations, this is a
-	 * compiler barrier as well.
 	 */
 	iret_to_self();
 }

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

* Re: [PATCH] x86/cpu: Fix typos and improve the comments in sync_core()
  2020-08-18  5:31   ` [PATCH] x86/cpu: Fix typos and improve the comments in sync_core() Ingo Molnar
@ 2020-08-19  1:00     ` Ricardo Neri
  2020-08-19  7:55       ` Ingo Molnar
  2020-08-19  7:59     ` [tip: x86/cpu] " tip-bot2 for Ingo Molnar
  1 sibling, 1 reply; 6+ messages in thread
From: Ricardo Neri @ 2020-08-19  1:00 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, linux-tip-commits, Andy Lutomirski,
	Borislav Petkov, Tony Luck, x86

On Tue, Aug 18, 2020 at 07:31:30AM +0200, Ingo Molnar wrote:
> 
> * tip-bot2 for Ricardo Neri <tip-bot2@linutronix.de> wrote:
> 
> > --- a/arch/x86/include/asm/sync_core.h
> > +++ b/arch/x86/include/asm/sync_core.h
> > @@ -5,6 +5,7 @@
> >  #include <linux/preempt.h>
> >  #include <asm/processor.h>
> >  #include <asm/cpufeature.h>
> > +#include <asm/special_insns.h>
> >  
> >  #ifdef CONFIG_X86_32
> >  static inline void iret_to_self(void)
> > @@ -54,14 +55,23 @@ static inline void iret_to_self(void)
> >  static inline void sync_core(void)
> >  {
> >  	/*
> > +	 * The SERIALIZE instruction is the most straightforward way to
> > +	 * do this but it not universally available.
> > +	 */
> > +	if (static_cpu_has(X86_FEATURE_SERIALIZE)) {
> > +		serialize();
> > +		return;
> > +	}
> > +
> > +	/*
> > +	 * For all other processors, there are quite a few ways to do this.
> > +	 * IRET-to-self is nice because it works on every CPU, at any CPL
> > +	 * (so it's compatible with paravirtualization), and it never exits
> > +	 * to a hypervisor. The only down sides are that it's a bit slow
> > +	 * (it seems to be a bit more than 2x slower than the fastest
> > +	 * options) and that it unmasks NMIs.  The "push %cs" is needed
> > +	 * because, in paravirtual environments, __KERNEL_CS may not be a
> > +	 * valid CS value when we do IRET directly.
> 
> So there's two typos in the new comments, there are at least two 
> misapplied commas, it departs from existing style, and there's a typo 
> in the existing comments as well.
> 
> Also, before this patch the 'compiler barrier' comment was valid for 
> the whole function (there was no branching), but after this patch it 
> reads of it was only valid for the legacy IRET-to-self branch.
> 
> Which together broke my detector and triggered a bit of compulsive 
> bike-shed painting. ;-) See the resulting patch below.
> 
> Thanks,
> 
> 	Ingo
> 
> ================>
> From: Ingo Molnar <mingo@kernel.org>
> Date: Tue, 18 Aug 2020 07:24:05 +0200
> Subject: [PATCH] x86/cpu: Fix typos and improve the comments in sync_core()
> 
> - Fix typos.
> 
> - Move the compiler barrier comment to the top, because it's valid for the
>   whole function, not just the legacy branch.
> 
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/include/asm/sync_core.h | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/sync_core.h b/arch/x86/include/asm/sync_core.h
> index 4631c0f969d4..0fd4a9dfb29c 100644
> --- a/arch/x86/include/asm/sync_core.h
> +++ b/arch/x86/include/asm/sync_core.h
> @@ -47,16 +47,19 @@ static inline void iret_to_self(void)
>   *
>   *  b) Text was modified on a different CPU, may subsequently be
>   *     executed on this CPU, and you want to make sure the new version
> - *     gets executed.  This generally means you're calling this in a IPI.
> + *     gets executed.  This generally means you're calling this in an IPI.
>   *
>   * If you're calling this for a different reason, you're probably doing
>   * it wrong.
> + *
> + * Like all of Linux's memory ordering operations, this is a
> + * compiler barrier as well.
>   */
>  static inline void sync_core(void)
>  {
>  	/*
>  	 * The SERIALIZE instruction is the most straightforward way to
> -	 * do this but it not universally available.
> +	 * do this, but it is not universally available.

Indeed, I missed this grammar error.

>  	 */
>  	if (static_cpu_has(X86_FEATURE_SERIALIZE)) {
>  		serialize();
> @@ -67,10 +70,10 @@ static inline void sync_core(void)
>  	 * For all other processors, there are quite a few ways to do this.
>  	 * IRET-to-self is nice because it works on every CPU, at any CPL
>  	 * (so it's compatible with paravirtualization), and it never exits
> -	 * to a hypervisor. The only down sides are that it's a bit slow
> +	 * to a hypervisor.  The only downsides are that it's a bit slow
>  	 * (it seems to be a bit more than 2x slower than the fastest
> -	 * options) and that it unmasks NMIs.  The "push %cs" is needed
> -	 * because, in paravirtual environments, __KERNEL_CS may not be a
> +	 * options) and that it unmasks NMIs.  The "push %cs" is needed,
> +	 * because in paravirtual environments __KERNEL_CS may not be a

I didn't realize that the double spaces after the period were part of the
style.

FWIW,

Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

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

* Re: [PATCH] x86/cpu: Fix typos and improve the comments in sync_core()
  2020-08-19  1:00     ` Ricardo Neri
@ 2020-08-19  7:55       ` Ingo Molnar
  0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2020-08-19  7:55 UTC (permalink / raw)
  To: Ricardo Neri
  Cc: linux-kernel, linux-tip-commits, Andy Lutomirski,
	Borislav Petkov, Tony Luck, x86


* Ricardo Neri <ricardo.neri-calderon@linux.intel.com> wrote:

> > @@ -47,16 +47,19 @@ static inline void iret_to_self(void)
> >   *
> >   *  b) Text was modified on a different CPU, may subsequently be
> >   *     executed on this CPU, and you want to make sure the new version
> > - *     gets executed.  This generally means you're calling this in a IPI.
> > + *     gets executed.  This generally means you're calling this in an IPI.
> >   *
> >   * If you're calling this for a different reason, you're probably doing
> >   * it wrong.
> > + *
> > + * Like all of Linux's memory ordering operations, this is a
> > + * compiler barrier as well.
> >   */
> >  static inline void sync_core(void)
> >  {
> >  	/*
> >  	 * The SERIALIZE instruction is the most straightforward way to
> > -	 * do this but it not universally available.
> > +	 * do this, but it is not universally available.
> 
> Indeed, I missed this grammar error.
> 
> >  	 */
> >  	if (static_cpu_has(X86_FEATURE_SERIALIZE)) {
> >  		serialize();
> > @@ -67,10 +70,10 @@ static inline void sync_core(void)
> >  	 * For all other processors, there are quite a few ways to do this.
> >  	 * IRET-to-self is nice because it works on every CPU, at any CPL
> >  	 * (so it's compatible with paravirtualization), and it never exits
> > -	 * to a hypervisor. The only down sides are that it's a bit slow
> > +	 * to a hypervisor.  The only downsides are that it's a bit slow

And this one - it's "downsides" not "down sides".

> >  	 * (it seems to be a bit more than 2x slower than the fastest
> > -	 * options) and that it unmasks NMIs.  The "push %cs" is needed
> > -	 * because, in paravirtual environments, __KERNEL_CS may not be a
> > +	 * options) and that it unmasks NMIs.  The "push %cs" is needed,
> > +	 * because in paravirtual environments __KERNEL_CS may not be a
> 
> I didn't realize that the double spaces after the period were part of the
> style.

They are not, but *consistent* use of typographic details is part of 
the style, and here we were mixing two styles within the same comment 
block.

> FWIW,
> 
> Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

Thanks,

	Ingo

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

* [tip: x86/cpu] x86/cpu: Fix typos and improve the comments in sync_core()
  2020-08-18  5:31   ` [PATCH] x86/cpu: Fix typos and improve the comments in sync_core() Ingo Molnar
  2020-08-19  1:00     ` Ricardo Neri
@ 2020-08-19  7:59     ` tip-bot2 for Ingo Molnar
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot2 for Ingo Molnar @ 2020-08-19  7:59 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Ingo Molnar, Ricardo Neri, x86, LKML

The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     40eb0cb4939e462acfedea8c8064571e886b9773
Gitweb:        https://git.kernel.org/tip/40eb0cb4939e462acfedea8c8064571e886b9773
Author:        Ingo Molnar <mingo@kernel.org>
AuthorDate:    Tue, 18 Aug 2020 07:31:30 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 19 Aug 2020 09:56:36 +02:00

x86/cpu: Fix typos and improve the comments in sync_core()

- Fix typos.

- Move the compiler barrier comment to the top, because it's valid for the
  whole function, not just the legacy branch.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20200818053130.GA3161093@gmail.com
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 arch/x86/include/asm/sync_core.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/sync_core.h b/arch/x86/include/asm/sync_core.h
index 4631c0f..0fd4a9d 100644
--- a/arch/x86/include/asm/sync_core.h
+++ b/arch/x86/include/asm/sync_core.h
@@ -47,16 +47,19 @@ static inline void iret_to_self(void)
  *
  *  b) Text was modified on a different CPU, may subsequently be
  *     executed on this CPU, and you want to make sure the new version
- *     gets executed.  This generally means you're calling this in a IPI.
+ *     gets executed.  This generally means you're calling this in an IPI.
  *
  * If you're calling this for a different reason, you're probably doing
  * it wrong.
+ *
+ * Like all of Linux's memory ordering operations, this is a
+ * compiler barrier as well.
  */
 static inline void sync_core(void)
 {
 	/*
 	 * The SERIALIZE instruction is the most straightforward way to
-	 * do this but it not universally available.
+	 * do this, but it is not universally available.
 	 */
 	if (static_cpu_has(X86_FEATURE_SERIALIZE)) {
 		serialize();
@@ -67,10 +70,10 @@ static inline void sync_core(void)
 	 * For all other processors, there are quite a few ways to do this.
 	 * IRET-to-self is nice because it works on every CPU, at any CPL
 	 * (so it's compatible with paravirtualization), and it never exits
-	 * to a hypervisor. The only down sides are that it's a bit slow
+	 * to a hypervisor.  The only downsides are that it's a bit slow
 	 * (it seems to be a bit more than 2x slower than the fastest
-	 * options) and that it unmasks NMIs.  The "push %cs" is needed
-	 * because, in paravirtual environments, __KERNEL_CS may not be a
+	 * options) and that it unmasks NMIs.  The "push %cs" is needed,
+	 * because in paravirtual environments __KERNEL_CS may not be a
 	 * valid CS value when we do IRET directly.
 	 *
 	 * In case NMI unmasking or performance ever becomes a problem,
@@ -81,9 +84,6 @@ static inline void sync_core(void)
 	 * CPUID is the conventional way, but it's nasty: it doesn't
 	 * exist on some 486-like CPUs, and it usually exits to a
 	 * hypervisor.
-	 *
-	 * Like all of Linux's memory ordering operations, this is a
-	 * compiler barrier as well.
 	 */
 	iret_to_self();
 }

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

end of thread, other threads:[~2020-08-19  7:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-07  3:28 [PATCH v4] x86/cpu: Use SERIALIZE in sync_core() when available Ricardo Neri
2020-08-17 15:47 ` [tip: x86/cpu] " tip-bot2 for Ricardo Neri
2020-08-18  5:31   ` [PATCH] x86/cpu: Fix typos and improve the comments in sync_core() Ingo Molnar
2020-08-19  1:00     ` Ricardo Neri
2020-08-19  7:55       ` Ingo Molnar
2020-08-19  7:59     ` [tip: x86/cpu] " tip-bot2 for Ingo Molnar

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.